Closed
Bug 1272653
Opened 8 years ago
Closed 8 years ago
Implement spec compatible Get Element Attribute and Get Element Property commands
Categories
(Remote Protocol :: Marionette, defect)
Tracking
(firefox48 fixed, firefox49 fixed)
RESOLVED
FIXED
mozilla49
People
(Reporter: ato, Assigned: ato)
References
(Blocks 1 open bug)
Details
(Keywords: pi-marionette-server)
Attachments
(5 files)
58 bytes,
text/x-review-board-request
|
jgriffin
:
review+
|
Details |
58 bytes,
text/x-review-board-request
|
jgriffin
:
review+
|
Details |
58 bytes,
text/x-review-board-request
|
jgriffin
:
review+
|
Details |
58 bytes,
text/x-review-board-request
|
automatedtester
:
review+
|
Details |
58 bytes,
text/x-review-board-request
|
automatedtester
:
review+
|
Details |
The current function GeckoDriver#getElementAttribute uses the Selenium atom that conflates attributes and properties. According to the WebDriver specification we should not conflate them any more. Get Element Attribute should get an element’s attribute, and not prioritise its property if there exists one with the same name: http://w3c.github.io/webdriver/webdriver-spec.html#dfn-get-element-attribute Get Element Property should get an element’s property, but not the attribute: http://w3c.github.io/webdriver/webdriver-spec.html#dfn-get-element-property
Assignee | ||
Updated•8 years ago
|
Assignee | ||
Comment 1•8 years ago
|
||
Renames test_elementState.py to test_element_state.py to patch Python naming conventions. Merges test_getattr*.py tests into test_element_state.py to match WebDriver specification chapter structure. Review commit: https://reviewboard.mozilla.org/r/52463/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/52463/
Attachment #8752196 -
Flags: review?(jgriffin)
Attachment #8752197 -
Flags: review?(jgriffin)
Attachment #8752198 -
Flags: review?(jgriffin)
Assignee | ||
Comment 2•8 years ago
|
||
Review commit: https://reviewboard.mozilla.org/r/52465/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/52465/
Assignee | ||
Comment 3•8 years ago
|
||
Review commit: https://reviewboard.mozilla.org/r/52467/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/52467/
Assignee | ||
Comment 4•8 years ago
|
||
Comment on attachment 8752196 [details] MozReview Request: Bug 1272653 - Merge element state tests; r?jgriffin Review request updated; see interdiff: https://reviewboard.mozilla.org/r/52463/diff/1-2/
Attachment #8752196 -
Attachment description: MozReview Request: Bug 1272653 - Restructure element state tests; r?jgriffin → MozReview Request: Bug 1272653 - Merge element state tests; r?jgriffin
Assignee | ||
Comment 5•8 years ago
|
||
Comment on attachment 8752197 [details] MozReview Request: Bug 1272653 - Restructure element state test cases; r?jgriffin Review request updated; see interdiff: https://reviewboard.mozilla.org/r/52465/diff/1-2/
Assignee | ||
Comment 6•8 years ago
|
||
Comment on attachment 8752198 [details] MozReview Request: Bug 1272653 - Implement WebDriver command Get Element Property; r?jgriffin Review request updated; see interdiff: https://reviewboard.mozilla.org/r/52467/diff/1-2/
Assignee | ||
Comment 7•8 years ago
|
||
The first three patches restructures the existing element state tests a bit and adds the Get Element Property command. I plan to adjust getElementAttribute to match the realities of the spec in an upcoming patch later tonight or quite possibly next week. This may require some backwards compatibility work because existing tests may rely on the Selenium idea of what an attribute is. It looks like this work will mean we can get ride of the atom.getElementAttribute atom from Selenium, which is good news.
Comment 8•8 years ago
|
||
Comment on attachment 8752196 [details] MozReview Request: Bug 1272653 - Merge element state tests; r?jgriffin https://reviewboard.mozilla.org/r/52463/#review49499
Attachment #8752196 -
Flags: review?(jgriffin) → review+
Updated•8 years ago
|
Attachment #8752197 -
Flags: review?(jgriffin) → review+
Comment 9•8 years ago
|
||
Comment on attachment 8752197 [details] MozReview Request: Bug 1272653 - Restructure element state test cases; r?jgriffin https://reviewboard.mozilla.org/r/52465/#review49503 ::: testing/marionette/harness/marionette/tests/unit/test_element_state_chrome.py:24 (Diff revision 2) > self.assertNotEqual(self.win, self.marionette.current_window_handle) > self.marionette.execute_script("window.close();") > self.marionette.switch_to_window(self.win) > MarionetteTestCase.tearDown(self) > > - def test_isEnabled(self): > + def test_enabeld(self): s/enabeld/enabled/
Comment 10•8 years ago
|
||
Comment on attachment 8752198 [details] MozReview Request: Bug 1272653 - Implement WebDriver command Get Element Property; r?jgriffin https://reviewboard.mozilla.org/r/52467/#review49505 lgtm!
Attachment #8752198 -
Flags: review?(jgriffin) → review+
Assignee | ||
Comment 11•8 years ago
|
||
Review commit: https://reviewboard.mozilla.org/r/52950/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/52950/
Attachment #8753034 -
Flags: review?(dburns)
Assignee | ||
Comment 12•8 years ago
|
||
Comment on attachment 8752196 [details] MozReview Request: Bug 1272653 - Merge element state tests; r?jgriffin Review request updated; see interdiff: https://reviewboard.mozilla.org/r/52463/diff/2-3/
Assignee | ||
Comment 13•8 years ago
|
||
Comment on attachment 8752197 [details] MozReview Request: Bug 1272653 - Restructure element state test cases; r?jgriffin Review request updated; see interdiff: https://reviewboard.mozilla.org/r/52465/diff/2-3/
Assignee | ||
Comment 14•8 years ago
|
||
Comment on attachment 8752198 [details] MozReview Request: Bug 1272653 - Implement WebDriver command Get Element Property; r?jgriffin Review request updated; see interdiff: https://reviewboard.mozilla.org/r/52467/diff/2-3/
Assignee | ||
Comment 15•8 years ago
|
||
Comment on attachment 8753034 [details] MozReview Request: Bug 1272653 - Make Get Element Attribute spec compatible; r?automatedtester Review request updated; see interdiff: https://reviewboard.mozilla.org/r/52950/diff/1-2/
Assignee | ||
Comment 16•8 years ago
|
||
There are a lot of test failures. I am assuming many of these stem from the fact that the last and final patch changes Get Element Attribute behaviour that existing Gecko tests rely on. I will investigate this further on Wednesday. (I have tomorrow off.)
Comment 17•8 years ago
|
||
Comment on attachment 8753034 [details] MozReview Request: Bug 1272653 - Make Get Element Attribute spec compatible; r?automatedtester https://reviewboard.mozilla.org/r/52950/#review50122
Attachment #8753034 -
Flags: review?(dburns) → review+
Assignee | ||
Comment 18•8 years ago
|
||
Review commit: https://reviewboard.mozilla.org/r/54588/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/54588/
Attachment #8755391 -
Flags: review?(dburns)
Assignee | ||
Comment 19•8 years ago
|
||
Comment on attachment 8752196 [details] MozReview Request: Bug 1272653 - Merge element state tests; r?jgriffin Review request updated; see interdiff: https://reviewboard.mozilla.org/r/52463/diff/3-4/
Assignee | ||
Comment 20•8 years ago
|
||
Comment on attachment 8752197 [details] MozReview Request: Bug 1272653 - Restructure element state test cases; r?jgriffin Review request updated; see interdiff: https://reviewboard.mozilla.org/r/52465/diff/3-4/
Assignee | ||
Comment 21•8 years ago
|
||
Comment on attachment 8752198 [details] MozReview Request: Bug 1272653 - Implement WebDriver command Get Element Property; r?jgriffin Review request updated; see interdiff: https://reviewboard.mozilla.org/r/52467/diff/3-4/
Assignee | ||
Comment 22•8 years ago
|
||
Comment on attachment 8753034 [details] MozReview Request: Bug 1272653 - Make Get Element Attribute spec compatible; r?automatedtester Review request updated; see interdiff: https://reviewboard.mozilla.org/r/52950/diff/2-3/
Comment 23•8 years ago
|
||
Comment on attachment 8755391 [details] MozReview Request: Bug 1272653 - Update tests to get property where appropriate; r?automatedtester https://reviewboard.mozilla.org/r/54588/#review51252
Attachment #8755391 -
Flags: review?(dburns) → review+
Assignee | ||
Comment 24•8 years ago
|
||
Comment on attachment 8752196 [details] MozReview Request: Bug 1272653 - Merge element state tests; r?jgriffin Review request updated; see interdiff: https://reviewboard.mozilla.org/r/52463/diff/4-5/
Assignee | ||
Comment 25•8 years ago
|
||
Comment on attachment 8752197 [details] MozReview Request: Bug 1272653 - Restructure element state test cases; r?jgriffin Review request updated; see interdiff: https://reviewboard.mozilla.org/r/52465/diff/4-5/
Assignee | ||
Comment 26•8 years ago
|
||
Comment on attachment 8752198 [details] MozReview Request: Bug 1272653 - Implement WebDriver command Get Element Property; r?jgriffin Review request updated; see interdiff: https://reviewboard.mozilla.org/r/52467/diff/4-5/
Assignee | ||
Comment 27•8 years ago
|
||
Comment on attachment 8753034 [details] MozReview Request: Bug 1272653 - Make Get Element Attribute spec compatible; r?automatedtester Review request updated; see interdiff: https://reviewboard.mozilla.org/r/52950/diff/3-4/
Assignee | ||
Comment 28•8 years ago
|
||
Comment on attachment 8755391 [details] MozReview Request: Bug 1272653 - Update tests to get property where appropriate; r?automatedtester Review request updated; see interdiff: https://reviewboard.mozilla.org/r/54588/diff/1-2/
Comment 29•8 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/685b7400fe1b https://hg.mozilla.org/integration/mozilla-inbound/rev/36021d89d5bf https://hg.mozilla.org/integration/mozilla-inbound/rev/2a6a5e53e03f https://hg.mozilla.org/integration/mozilla-inbound/rev/a8d2a36786aa https://hg.mozilla.org/integration/mozilla-inbound/rev/ee21562d144c
Comment 30•8 years ago
|
||
The landing of this patch perma broke our firefox-ui-tests! See bug 1275249 for details.
Comment 31•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/685b7400fe1b https://hg.mozilla.org/mozilla-central/rev/36021d89d5bf https://hg.mozilla.org/mozilla-central/rev/2a6a5e53e03f https://hg.mozilla.org/mozilla-central/rev/a8d2a36786aa https://hg.mozilla.org/mozilla-central/rev/ee21562d144c
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
status-firefox49:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla49
Comment 34•8 years ago
|
||
bugherder uplift |
https://hg.mozilla.org/releases/mozilla-aurora/rev/06e08681af3a https://hg.mozilla.org/releases/mozilla-aurora/rev/83c3f083db41 https://hg.mozilla.org/releases/mozilla-aurora/rev/28dbd7065041
status-firefox48:
--- → fixed
Comment 35•8 years ago
|
||
bugherder uplift |
https://hg.mozilla.org/releases/mozilla-aurora/rev/06e08681af3a https://hg.mozilla.org/releases/mozilla-aurora/rev/83c3f083db41 https://hg.mozilla.org/releases/mozilla-aurora/rev/28dbd7065041
Updated•1 year ago
|
Product: Testing → Remote Protocol
You need to log in
before you can comment on or make changes to this bug.
Description
•