Closed
Bug 1394881
Opened 7 years ago
Closed 7 years ago
Replace element.isDisconnected with Node#isConnected
Categories
(Remote Protocol :: Marionette, enhancement, P1)
Tracking
(firefox57 fixed)
RESOLVED
FIXED
mozilla57
Tracking | Status | |
---|---|---|
firefox57 | --- | fixed |
People
(Reporter: ato, Assigned: ato)
References
Details
Attachments
(1 file)
It turns out that Node#isConnected (described in https://dom.spec.whatwg.org/#dom-node-isconnected) handles an element’s shadow root, which is exactly the reason why we wrote element.isDisconnected in the first place. element.isDisconnected is long and error-prone and can be removed entirely in favour of this web platform API.
Assignee | ||
Updated•7 years ago
|
Comment hidden (mozreview-request) |
Assignee | ||
Updated•7 years ago
|
Attachment #8902678 -
Flags: review?(dburns)
Assignee | ||
Comment 2•7 years ago
|
||
One of the headaches of improving window tracking in Marionette has been how to deal element.Store#get taking a container object with references to WindowProxy and the shadowRoot element. By using isConnected on the element reference itself, we can do away with passing these into the known element store (or store references to them internally in the store).
Blocks: marionette-window-tracking
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Assignee | ||
Updated•7 years ago
|
Priority: -- → P1
Comment 5•7 years ago
|
||
mozreview-review |
Comment on attachment 8902678 [details] Bug 1394881 - Use Node.isConnected for web element staleness check. https://reviewboard.mozilla.org/r/174352/#review179710 ::: testing/marionette/element.js:171 (Diff revision 3) > * the DOM provided in the container. > */ > - get(uuid, container) { > + get(uuid) { > let el = this.els[uuid]; > if (!el) { > - throw new NoSuchElementError(`Element reference not seen before: ` + > + throw new NoSuchElementError("Element reference not seen before: " + uuid); string templates would be neater here and matches the other errors (even though they have pprint)
Attachment #8902678 -
Flags: review?(dburns) → review+
Assignee | ||
Comment 6•7 years ago
|
||
mozreview-review-reply |
Comment on attachment 8902678 [details] Bug 1394881 - Use Node.isConnected for web element staleness check. https://reviewboard.mozilla.org/r/174352/#review179710 > string templates would be neater here and matches the other errors (even though they have pprint) I will address this in a follow-up as it’s quite crucial for my progress on the window tracking bug to get this landed on central. Assuming this is OK with you.
Comment 7•7 years ago
|
||
mozreview-review-reply |
Comment on attachment 8902678 [details] Bug 1394881 - Use Node.isConnected for web element staleness check. https://reviewboard.mozilla.org/r/174352/#review179710 > I will address this in a follow-up as it’s quite crucial for my progress on the window tracking bug to get this landed on central. Assuming this is OK with you. Thats fine with me
Pushed by atolfsen@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/d73e79c0d9d4 Use Node.isConnected for web element staleness check. r=automatedtester
Comment 9•7 years ago
|
||
This patch doesn't fix the issues with test_submit_unencrypted_info_warning.py, because the try build still doesn't contain any fx-ui tests: https://treeherder.mozilla.org/#/jobs?repo=try&revision=d9cd8b24b175&selectedJob=127101165 I filed bug 1395469 for the new signature.
Comment 10•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/d73e79c0d9d4
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
status-firefox57:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
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
•