1265854 - replace uses of nsIDOMNode constants

Status: RESOLVED FIXED

(DevTools :: Framework, enhancement, P1)

Description

[Tom Tromey :tromey]

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.

Comment 1

[James Long (:jlongster)]

Attachment: 1265854.patch

Comment 2

[Tom Tromey :tromey]

Comment on attachment 8754879 [details] [diff] [review]
1265854.patch

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.

Comment 3

[James Long (:jlongster)]

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.

Comment 4

[James Long (:jlongster)]

Couldn't push to try before, just got it working: https://treeherder.mozilla.org/#/jobs?repo=try&revision=4193be266b40 will still think about the loader idea

Comment 5

[James Long (:jlongster)]

New push: https://treeherder.mozilla.org/#/jobs?repo=try&revision=bca2e8b45fe0

Comment 6

[James Long (:jlongster)]

New push: https://treeherder.mozilla.org/#/jobs?repo=try&revision=501b40ea5b7f

Comment 7

[James Long (:jlongster)]

Attachment: 1265854.patch

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.

Comment 8

[James Long (:jlongster)]

Attachment: 1265854.patch

rebased

Comment 9

[James Long (:jlongster)]

Meant to do this earlier; previous try run had a ton of unrelated (I hope) failures: https://treeherder.mozilla.org/#/jobs?repo=try&revision=236b3121826b

Comment 10

[James Long (:jlongster)]

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: https://treeherder.mozilla.org/#/jobs?repo=try&revision=22088c0f66cd

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/fx-team/rev/be6fddfc1390
replace uses of nsiDOMNode constants in devtools frontend r=tromey

Comment 12

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/be6fddfc1390