Closed
Bug 1455676
Opened 6 years ago
Closed 6 years ago
Remove nsIDOMNode
Categories
(Core :: DOM: Core & HTML, enhancement, P2)
Core
DOM: Core & HTML
Tracking
()
RESOLVED
FIXED
mozilla62
Tracking | Status | |
---|---|---|
firefox62 | --- | fixed |
People
(Reporter: bzbarsky, Assigned: bzbarsky)
References
(Blocks 1 open bug)
Details
Attachments
(21 files)
No description provided.
Updated•6 years ago
|
Priority: -- → P2
Assignee | ||
Comment 1•6 years ago
|
||
Attachment #8980923 -
Flags: review?(kyle)
Assignee | ||
Updated•6 years ago
|
Assignee: nobody → bzbarsky
Status: NEW → ASSIGNED
Assignee | ||
Comment 2•6 years ago
|
||
Attachment #8980924 -
Flags: review?(surkov.alexander)
Assignee | ||
Comment 3•6 years ago
|
||
Attachment #8980925 -
Flags: review?(kyle)
Assignee | ||
Comment 4•6 years ago
|
||
Attachment #8980926 -
Flags: review?(kyle)
Assignee | ||
Comment 5•6 years ago
|
||
Attachment #8980927 -
Flags: review?(kyle)
Assignee | ||
Comment 6•6 years ago
|
||
Attachment #8980928 -
Flags: review?(kyle)
Assignee | ||
Comment 7•6 years ago
|
||
Attachment #8980929 -
Flags: review?(kyle)
Assignee | ||
Comment 8•6 years ago
|
||
Attachment #8980930 -
Flags: review?(kyle)
Assignee | ||
Comment 9•6 years ago
|
||
Attachment #8980931 -
Flags: review?(snorp)
Assignee | ||
Comment 10•6 years ago
|
||
Attachment #8980932 -
Flags: review?(kyle)
Assignee | ||
Comment 11•6 years ago
|
||
Attachment #8980933 -
Flags: review?(kyle)
Assignee | ||
Comment 12•6 years ago
|
||
Attachment #8980934 -
Flags: review?(kyle)
Assignee | ||
Comment 13•6 years ago
|
||
This needs pretty careful review for the update service changes, though I think they're pretty mechanical and safe.
Attachment #8980935 -
Flags: review?(dtownsend)
Assignee | ||
Comment 14•6 years ago
|
||
Attachment #8980936 -
Flags: review?(kyle)
Assignee | ||
Comment 15•6 years ago
|
||
Attachment #8980937 -
Flags: review?(surkov.alexander)
Assignee | ||
Comment 16•6 years ago
|
||
Attachment #8980938 -
Flags: review?(dtownsend)
Assignee | ||
Comment 17•6 years ago
|
||
Attachment #8980939 -
Flags: review?(ttromey)
Assignee | ||
Comment 18•6 years ago
|
||
Attachment #8980940 -
Flags: review?(mrbkap)
Assignee | ||
Comment 19•6 years ago
|
||
Attachment #8980941 -
Flags: review?(kyle)
Assignee | ||
Comment 20•6 years ago
|
||
Attachment #8980942 -
Flags: review?(kyle)
Assignee | ||
Comment 21•6 years ago
|
||
Attachment #8980943 -
Flags: review?(kyle)
Comment 22•6 years ago
|
||
Comment on attachment 8980924 [details] [diff] [review] part 2. Remove nsIDOMNode usage from accessible/ Review of attachment 8980924 [details] [diff] [review]: ----------------------------------------------------------------- there's dom nsFocusManager change in the patch, which looks good with me, but not in a11y module. You might need somebody else to check it.
Attachment #8980924 -
Flags: review?(surkov.alexander) → review+
Comment 23•6 years ago
|
||
Comment on attachment 8980937 [details] [diff] [review] part 15. Stop using nsIDOMNode in the a11y talos tests Review of attachment 8980937 [details] [diff] [review]: ----------------------------------------------------------------- we might not need all tree methods, if only ID version method is used
Attachment #8980937 -
Flags: review?(surkov.alexander) → review+
Assignee | ||
Comment 24•6 years ago
|
||
Comment on attachment 8980924 [details] [diff] [review] part 2. Remove nsIDOMNode usage from accessible/ Probably a good idea... Olli, can you review the nsFocusManager bit of this patch, please?
Attachment #8980924 -
Flags: review?(bugs)
Assignee | ||
Comment 25•6 years ago
|
||
> we might not need all tree methods, if only ID version method is used
The others are basically helpers for the id version which call themselves recursively.
Updated•6 years ago
|
Attachment #8980924 -
Flags: review?(bugs) → review+
Comment 26•6 years ago
|
||
Comment on attachment 8980939 [details] [diff] [review] part 17. Stop using nsIDOMNode in devtools/ I think someone more active in devtools should review this one.
Attachment #8980939 -
Flags: review?(ttromey) → review?(jdescottes)
Attachment #8980931 -
Flags: review?(snorp) → review+
Comment 27•6 years ago
|
||
Comment on attachment 8980939 [details] [diff] [review] part 17. Stop using nsIDOMNode in devtools/ Review of attachment 8980939 [details] [diff] [review]: ----------------------------------------------------------------- Looks good to me. Two nits (up to you), one file to remove from the diff. ::: devtools/client/shared/widgets/view-helpers.js @@ +312,5 @@ > > /** > * A generic Item is used to describe children present in a Widget. > * > + * This is basically a very thin wrapper around an Node, with a few nit: there are a few "an Node" in this patch, could we replace " an node" -> " a node" everywhere? ::: devtools/shared/builtin-modules.js @@ +36,5 @@ > Element, > Event, > FileReader, > FormData, > + Node, nit: could we keep the list sorted and move this before TextDecoder. Same comment for the next list (even though "InspectorUtils" is in the wrong spot in the second list) ::: package-lock.json @@ -1,1 @@ > { this file was probably updated as a side effect (after running eslint?) and should not be in this changeset
Attachment #8980939 -
Flags: review?(jdescottes) → review+
Assignee | ||
Comment 28•6 years ago
|
||
> nit: there are a few "an Node" in this patch, could we replace " an node" -> " a node" everywhere? Good catch! I thought I checked those after doing the search+replace, but clearly not well enough. > nit: could we keep the list sorted and move this before TextDecoder. Yes. I tried to do that, and clearly failed.... > this file was probably updated as a side effect (after running eslint?) Ugh, indeed. I fixed that once, but eslint is a gift that just keeps on giving. I filed bug 1465164 on that. Excellent catch.
Updated•6 years ago
|
Attachment #8980935 -
Flags: review?(dtownsend) → review+
Updated•6 years ago
|
Attachment #8980938 -
Flags: review?(dtownsend) → review+
Updated•6 years ago
|
Attachment #8980923 -
Flags: review?(kyle) → review+
Updated•6 years ago
|
Attachment #8980940 -
Flags: review?(mrbkap) → review+
Updated•6 years ago
|
Attachment #8980925 -
Flags: review?(kyle) → review+
Updated•6 years ago
|
Attachment #8980926 -
Flags: review?(kyle) → review+
Updated•6 years ago
|
Attachment #8980927 -
Flags: review?(kyle) → review+
Updated•6 years ago
|
Attachment #8980928 -
Flags: review?(kyle) → review+
Updated•6 years ago
|
Attachment #8980929 -
Flags: review?(kyle) → review+
Updated•6 years ago
|
Attachment #8980930 -
Flags: review?(kyle) → review+
Updated•6 years ago
|
Attachment #8980932 -
Flags: review?(kyle) → review+
Updated•6 years ago
|
Attachment #8980933 -
Flags: review?(kyle) → review+
Updated•6 years ago
|
Attachment #8980934 -
Flags: review?(kyle) → review+
Updated•6 years ago
|
Attachment #8980936 -
Flags: review?(kyle) → review+
Updated•6 years ago
|
Attachment #8980941 -
Flags: review?(kyle) → review+
Updated•6 years ago
|
Attachment #8980942 -
Flags: review?(kyle) → review+
Comment 29•6 years ago
|
||
Comment on attachment 8980943 [details] [diff] [review] part 21. Remove nsIDOMNode Review of attachment 8980943 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/base/test/test_bug352728.html @@ +50,5 @@ > var comment = document.createComment(aText); > var types = [ Comment, CharacterData, Node ]; > checkTypes(comment, "comment", types); > > + var interfaces = []; nit: here and below, this passes to a function that runs a for-loop on the interfaces array, so it's basically a noop now. Can be removed. ::: dom/base/test/test_bug352728.xhtml @@ +74,5 @@ > var comment = document.createComment(aText); > var types = [ Comment, CharacterData, Node ]; > checkTypes(comment, "comment", types); > > + var interfaces = []; nit: here and below, this passes to a function that runs a for-loop on the interfaces array, so it's basically a noop now. Can be removed.
Attachment #8980943 -
Flags: review?(kyle) → review+
Comment 30•6 years ago
|
||
Pushed by bzbarsky@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/c38176356708 part 1. Make it possible to importGlobalProperties Node. r=qdot https://hg.mozilla.org/integration/mozilla-inbound/rev/1ccf1348f689 part 2. Remove nsIDOMNode usage from accessible/. r=surkov https://hg.mozilla.org/integration/mozilla-inbound/rev/7161cb18122e part 3. Remove nsIDOMNode usage from widget/. r=qdot https://hg.mozilla.org/integration/mozilla-inbound/rev/5d81f28df4d2 part 4. Remove nsIDOMNode usage from uriloader/. r=qdot https://hg.mozilla.org/integration/mozilla-inbound/rev/55a0726047bb part 5. Remove nsIDOMNode usage from netwerk/. r=qdot https://hg.mozilla.org/integration/mozilla-inbound/rev/9013597c936e part 6. Remove nsIDOMNode usage from docshell/. r=qdot https://hg.mozilla.org/integration/mozilla-inbound/rev/4eb337cfade1 part 7. Remove nsIDOMNode usage from layout/inspector/. r=qdot https://hg.mozilla.org/integration/mozilla-inbound/rev/7e6146338754 part 8. Remove nsIDOMNode usage from layout/. r=qdot https://hg.mozilla.org/integration/mozilla-inbound/rev/62ece2b265ab part 9. Mostly remove use of nsIDOMNode from mobile/. r=snorp https://hg.mozilla.org/integration/mozilla-inbound/rev/94ab3582901b part 10. Remove use of nsIDOMNode from remaining xpidl. r=qdot https://hg.mozilla.org/integration/mozilla-inbound/rev/c6e11e7c543c part 11. Remove use of nsIDOMNode from xpfe/. r=qdot https://hg.mozilla.org/integration/mozilla-inbound/rev/220898648f5f part 12. Remove use of nsIDOMNode from xpcom/. r=qdot https://hg.mozilla.org/integration/mozilla-inbound/rev/3b02b634a18a part 13. Remove use of nsIDOMNode from toolkit/. r=mossop https://hg.mozilla.org/integration/mozilla-inbound/rev/e46b5b302771 part 14. Remove most use of nsIDOMNode in dom/. r=qdot https://hg.mozilla.org/integration/mozilla-inbound/rev/a81f7f1c0303 part 15. Stop using nsIDOMNode in the a11y talos tests. r=surkov https://hg.mozilla.org/integration/mozilla-inbound/rev/2e33b4da5aa6 part 16. Mostly stop using nsIDOMNode in browser/. r=mossop https://hg.mozilla.org/integration/mozilla-inbound/rev/bcf0bd0ee7bb part 17. Stop using nsIDOMNode in devtools/. r=jdescottes https://hg.mozilla.org/integration/mozilla-inbound/rev/8210471a0b5e part 18. Stop using getInterface as the primary API for the indexeddb permission notifications. r=mrbkap https://hg.mozilla.org/integration/mozilla-inbound/rev/94070be7a406 part 19. Remove some remaining nsIDOMNode uses. r=qdot https://hg.mozilla.org/integration/mozilla-inbound/rev/5eb989e189c7 part 20. Remove now-unused AsDOMNode methods. r=qdot https://hg.mozilla.org/integration/mozilla-inbound/rev/26548ddbd8d4 part 21. Remove nsIDOMNode. r=qdot
Comment 31•6 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/c38176356708 https://hg.mozilla.org/mozilla-central/rev/1ccf1348f689 https://hg.mozilla.org/mozilla-central/rev/7161cb18122e https://hg.mozilla.org/mozilla-central/rev/5d81f28df4d2 https://hg.mozilla.org/mozilla-central/rev/55a0726047bb https://hg.mozilla.org/mozilla-central/rev/9013597c936e https://hg.mozilla.org/mozilla-central/rev/4eb337cfade1 https://hg.mozilla.org/mozilla-central/rev/7e6146338754 https://hg.mozilla.org/mozilla-central/rev/62ece2b265ab https://hg.mozilla.org/mozilla-central/rev/94ab3582901b https://hg.mozilla.org/mozilla-central/rev/c6e11e7c543c https://hg.mozilla.org/mozilla-central/rev/220898648f5f https://hg.mozilla.org/mozilla-central/rev/3b02b634a18a https://hg.mozilla.org/mozilla-central/rev/e46b5b302771 https://hg.mozilla.org/mozilla-central/rev/a81f7f1c0303 https://hg.mozilla.org/mozilla-central/rev/2e33b4da5aa6 https://hg.mozilla.org/mozilla-central/rev/bcf0bd0ee7bb https://hg.mozilla.org/mozilla-central/rev/8210471a0b5e https://hg.mozilla.org/mozilla-central/rev/94070be7a406 https://hg.mozilla.org/mozilla-central/rev/5eb989e189c7 https://hg.mozilla.org/mozilla-central/rev/26548ddbd8d4
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
status-firefox62:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla62
Updated•5 years ago
|
Component: DOM → DOM: Core & HTML
You need to log in
before you can comment on or make changes to this bug.
Description
•