Closed Bug 1272653 Opened 9 years ago Closed 9 years ago

Implement spec compatible Get Element Attribute and Get Element Property commands

Categories

(Remote Protocol :: Marionette, defect)

Product:
Remote Protocol
Component:
Marionette
Version:
Version 3
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

(firefox48 fixed, firefox49 fixed)

Status:
RESOLVED FIXED
Milestone:
mozilla49
Tracking Flags:
Tracking Status
firefox48 --- fixed
firefox49 --- fixed

People

(Reporter: ato, Assigned: ato)

References

(Blocks 1 open bug)

Details

(Keywords: pi-marionette-server)

Attachments

(5 files)

MozReview Request: Bug 1272653 - Merge element state tests; r?jgriffin
58 bytes, text/x-review-board-request
jgriffin
: review+
Details
MozReview Request: Bug 1272653 - Restructure element state test cases; r?jgriffin
58 bytes, text/x-review-board-request
jgriffin
: review+
Details
MozReview Request: Bug 1272653 - Implement WebDriver command Get Element Property; r?jgriffin
58 bytes, text/x-review-board-request
jgriffin
: review+
Details
MozReview Request: Bug 1272653 - Make Get Element Attribute spec compatible; r?automatedtester
58 bytes, text/x-review-board-request
automatedtester
: review+
Details
MozReview Request: Bug 1272653 - Update tests to get property where appropriate; r?automatedtester
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: nobody → ato
Status: NEW → ASSIGNED
Keywords: ateam-marionette-server
Blocks: 1260233
Blocks: webdriver
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)
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
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/
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/
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 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+
Attachment #8752197 - Flags: review?(jgriffin) → review+
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 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+
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/
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/
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/
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/
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 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+
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/
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/
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/
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 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+
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/
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/
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/
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/
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/
The landing of this patch perma broke our firefox-ui-tests! See bug 1275249 for details.
Depends on: 1276182
Product: Testing → Remote Protocol
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: