Closed Bug 1288863 Opened 8 years ago Closed 8 years ago

Capabilities are not returned as lower case.

Categories

(Remote Protocol :: Marionette, defect)

defect
Not set
normal

Tracking

(firefox49 fixed, firefox50 fixed, firefox51 fixed)

RESOLVED FIXED
mozilla51
Tracking Status
firefox49 --- fixed
firefox50 --- fixed
firefox51 --- fixed

People

(Reporter: automatedtester, Assigned: automatedtester)

References

()

Details

(Keywords: pi-marionette-server)

Attachments

(2 files)

Assignee: nobody → dburns
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/
Attachment #8773994 - Flags: review?(ato)
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.
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/
Attachment #8774655 - Flags: review?(hskupin)
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/
Attachment #8773994 - Flags: review?(ato) → review+
Comment on attachment 8773994 [details] Bug 1288863: Return platformName and browserVersion as lowercase https://reviewboard.mozilla.org/r/66618/#review63968
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>
Attachment #8774655 - Flags: review?(hskupin) → review-
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.
Flags: needinfo?(hskupin)
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.
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/
Attachment #8774655 - Flags: review- → review?(hskupin)
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/
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/
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/
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.
Attachment #8774655 - Flags: review?(hskupin) → review+
Flags: needinfo?(hskupin)
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
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla51
Whiteboard: [checkin-needed-aurora][checkin-needed-beta]
Whiteboard: [checkin-needed-aurora][checkin-needed-beta] → [checkin-needed-beta]
Product: Testing → Remote Protocol
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: