Closed Bug 1265854 Opened 4 years ago Closed 4 years ago

replace uses of nsIDOMNode constants


(DevTools :: Framework, enhancement, P1)



(firefox49 fixed)

Firefox 49
49.3 - Jun 6
Tracking Status
firefox49 --- fixed


(Reporter: tromey, Assigned: jlong)



(Whiteboard: [devtools-html])


(1 file, 2 obsolete files)

In the inspector at least, nsIDOMNode seems to be used only for constants.
These uses need to be replaced for the devtools de-chrome-ification project.
devtools.html did this by:

    nsIDOMNode: HTMLElement,

so perhaps this idea can be reused.
Assignee: nobody → jlong
Iteration: --- → 49.1 - May 9
Flags: qe-verify-
Priority: -- → P1
Iteration: 49.1 - May 9 → 49.2 - May 23
Attached patch 1265854.patch (obsolete) — Splinter Review
Attachment #8754879 - Flags: review?(ttromey)
Comment on attachment 8754879 [details] [diff] [review]

Review of attachment 8754879 [details] [diff] [review]:

Thank you for doing this.

I think this is all good, but I wonder if it would be simpler and more webbish to just define "Node" globally in Loader.jsm as nsIDOMNode; and then use Node.SOME_CONSTANT everywhere.  The developer-toolbar.js change gave me this idea.
Attachment #8754879 - Flags: review?(ttromey) → review+
That's not a bad idea, although we've been trying to minimize the loader magic. Ideally, everything would be loaded through BrowserLoader and we could just have direct access to those APIs. But I'll think about the loader approach.
Couldn't push to try before, just got it working: will still think about the loader idea
Iteration: 49.2 - May 23 → 49.3 - Jun 6
Attached patch 1265854.patch (obsolete) — Splinter Review
I'm hesistant to touch Loader.jsm. That's been a source pain several times. I'd rather not keep "faking" a web environment, and this approach is pretty simple so I think we should continue with it.

This patch fixes a few last things. If the try run is green I will land it.
Attached patch 1265854.patchSplinter Review
Attachment #8754879 - Attachment is obsolete: true
Attachment #8757502 - Attachment is obsolete: true
Meant to do this earlier; previous try run had a ton of unrelated (I hope) failures:
There's been a lot of unrelated errors on try. But I saw one bustage that was my fault. Going to do another run just in case:
Pushed by
replace uses of nsiDOMNode constants in devtools frontend r=tromey
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 49
Depends on: 1305416
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.