Closed
Bug 1265854
Opened 8 years ago
Closed 8 years ago
replace uses of nsIDOMNode constants
Categories
(DevTools :: Framework, enhancement, P1)
DevTools
Framework
Tracking
(firefox49 fixed)
Tracking | Status | |
---|---|---|
firefox49 | --- | fixed |
People
(Reporter: tromey, Assigned: jlong)
References
Details
(Whiteboard: [devtools-html])
Attachments
(1 file, 2 obsolete files)
(deleted),
patch
|
Details | Diff | Splinter Review |
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.
Updated•8 years ago
|
Assignee: nobody → jlong
Status: NEW → ASSIGNED
Iteration: --- → 49.1 - May 9
Flags: qe-verify-
Priority: -- → P1
Updated•8 years ago
|
Iteration: 49.1 - May 9 → 49.2 - May 23
Assignee | ||
Comment 1•8 years ago
|
||
Attachment #8754879 -
Flags: review?(ttromey)
Reporter | ||
Comment 2•8 years ago
|
||
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.
Attachment #8754879 -
Flags: review?(ttromey) → review+
Assignee | ||
Comment 3•8 years ago
|
||
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.
Assignee | ||
Comment 4•8 years ago
|
||
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
Updated•8 years ago
|
Iteration: 49.2 - May 23 → 49.3 - Jun 6
Assignee | ||
Comment 5•8 years ago
|
||
New push: https://treeherder.mozilla.org/#/jobs?repo=try&revision=bca2e8b45fe0
Assignee | ||
Comment 6•8 years ago
|
||
New push: https://treeherder.mozilla.org/#/jobs?repo=try&revision=501b40ea5b7f
Assignee | ||
Comment 7•8 years ago
|
||
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.
Assignee | ||
Comment 8•8 years ago
|
||
rebased
Attachment #8754879 -
Attachment is obsolete: true
Attachment #8757502 -
Attachment is obsolete: true
Assignee | ||
Comment 9•8 years ago
|
||
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
Assignee | ||
Comment 10•8 years ago
|
||
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•8 years ago
|
||
Pushed by jlong@mozilla.com: https://hg.mozilla.org/integration/fx-team/rev/be6fddfc1390 replace uses of nsiDOMNode constants in devtools frontend r=tromey
Comment 12•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/be6fddfc1390
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
status-firefox49:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 49
Updated•6 years ago
|
Product: Firefox → DevTools
You need to log in
before you can comment on or make changes to this bug.
Description
•