David Burns :automatedtester Assignee
	Description • 8 years ago

This is being raised from https://github.com/mozilla/geckodriver/issues/148

	David Burns :automatedtester Assignee
	Comment 1 • 8 years ago

Currently Marionette returns directly from appInfo where the webdriver
specification mandates that we return lowercase for those. See
http://w3c.github.io/webdriver/webdriver-spec.html#capabilities

Review commit: https://reviewboard.mozilla.org/r/66618/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/66618/

	Henrik Skupin [:whimboo][⌚️UTC+1]
	Comment 2 • 8 years ago

https://reviewboard.mozilla.org/r/66618/#review63562

::: testing/marionette/driver.js:133
(Diff revision 1)
>  
>    this.sessionCapabilities = {
>      // mandated capabilities
> -    "browserName": Services.appinfo.name,
> +    "browserName": Services.appinfo.name.toLowerCase(),
>      "browserVersion": Services.appinfo.version,
> -    "platformName": Services.sysinfo.getProperty("name"),
> +    "platformName": Services.sysinfo.getProperty("name").toLowerCase(),

Can you please make sure to also fix firefox-puppeteer modules which make use of platform? Given that we don't run on platforms other than Linux you don't see failures on try. Thanks.

	David Burns :automatedtester Assignee
	Comment 3 • 8 years ago

The capabilities, according to the webdriver specification, should all be lowercase.

Review commit: https://reviewboard.mozilla.org/r/67108/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/67108/

	David Burns :automatedtester Assignee
	Comment 4 • 8 years ago

Comment on attachment 8773994 [details]
Bug 1288863: Return platformName and browserVersion as lowercase

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/66618/diff/1-2/

	Andreas Tolfsen ❲:ato❳
	Comment 5 • 8 years ago

Comment on attachment 8773994 [details]
Bug 1288863: Return platformName and browserVersion as lowercase

https://reviewboard.mozilla.org/r/66618/#review63968

	Henrik Skupin [:whimboo][⌚️UTC+1]
	Comment 6 • 8 years ago

Comment on attachment 8774655 [details]
Bug 1288863: Update Firefox Puppeteer to use lower case platformName

https://reviewboard.mozilla.org/r/67108/#review63982

<p>With this change we should also remove the extra calls to <code>lower()</code> which are not necessary anymore:<br />
https://dxr.mozilla.org/mozilla-central/search?q=platformName+path%3Atesting%2Fpuppeteer&amp;redirect=true</p>
<p>Also please check the following link for other necessary changes:<br />
https://dxr.mozilla.org/mozilla-central/search?q=path%3Atesting%2Fpuppeteer+platform&amp;redirect=false</p>

	David Burns :automatedtester Assignee
	Comment 7 • 8 years ago

https://reviewboard.mozilla.org/r/67108/#review63982

Those were left as I thought they might be used with update tests which means if it hit a previous version it wouldnt go down the right path. Do we use any of those methods in update tests?

As for the 2nd link the only thing that hasnt been updated is the use of `lower()` which I will remove once you have clarified if they are used in update tests.

	David Burns :automatedtester Assignee
	Comment 8 • 8 years ago

See comment 7

	Henrik Skupin [:whimboo][⌚️UTC+1]
	Comment 9 • 8 years ago

https://reviewboard.mozilla.org/r/67108/#review63982

Thanks for caring about the update tests! So in general we only use `platformName` to determine the feature set of shortcuts for various shortcuts. The update tests don't make use of shortcuts but menu entries and clicks to e.g. open the about window. Because of that we should be safe to change all of those entries. 

You can verify that after your changes by running `mach firefox-ui-update --binary %old-nightly-build% --update-fallback-only`. It should not fail even with other capitalization of the platform name.

	David Burns :automatedtester Assignee
	Comment 10 • 8 years ago

Comment on attachment 8773994 [details]
Bug 1288863: Return platformName and browserVersion as lowercase

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/66618/diff/2-3/

	David Burns :automatedtester Assignee
	Comment 11 • 8 years ago

Comment on attachment 8774655 [details]
Bug 1288863: Update Firefox Puppeteer to use lower case platformName

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/67108/diff/1-2/

	David Burns :automatedtester Assignee
	Comment 12 • 8 years ago

Comment on attachment 8773994 [details]
Bug 1288863: Return platformName and browserVersion as lowercase

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/66618/diff/3-4/

	David Burns :automatedtester Assignee
	Comment 13 • 8 years ago

Comment on attachment 8774655 [details]
Bug 1288863: Update Firefox Puppeteer to use lower case platformName

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/67108/diff/2-3/

	Henrik Skupin [:whimboo][⌚️UTC+1]
	Comment 14 • 8 years ago

Comment on attachment 8774655 [details]
Bug 1288863: Update Firefox Puppeteer to use lower case platformName

https://reviewboard.mozilla.org/r/67108/#review65990

I still miss an update for the following code:
https://dxr.mozilla.org/mozilla-central/source/testing/puppeteer/firefox/firefox_puppeteer/ui/browser/window.py#258

In that case I would say we can really get rid of the if completely. If a test wants to make use of a shortcut on Windows we would fail in determining the DTD entity or fail opening the window which would give a clear message either way.

r=me with both lines removed.

	Pulsebot
	Comment 15 • 8 years ago

Pushed by dburns@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/6f7e63905686
Return platformName and browserVersion as lowercase r=ato
https://hg.mozilla.org/integration/autoland/rev/6fccb3cdd763
Update Firefox Puppeteer to use lower case platformName r=whimboo

	Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)
	Comment 16 • 8 years ago
bugherder

https://hg.mozilla.org/mozilla-central/rev/6f7e63905686
https://hg.mozilla.org/mozilla-central/rev/6fccb3cdd763

	Carsten Book [:Tomcat]
	Comment 17 • 8 years ago
bugherder uplift

https://hg.mozilla.org/releases/mozilla-aurora/rev/7a088d9630fc
https://hg.mozilla.org/releases/mozilla-aurora/rev/bd95b171b2e3

	Carsten Book [:Tomcat]
	Comment 18 • 8 years ago
bugherder uplift

https://hg.mozilla.org/releases/mozilla-beta/rev/fb620cf5aae2
https://hg.mozilla.org/releases/mozilla-beta/rev/6106148d2caa