Closed Bug 1390209 Opened 2 years ago Closed 2 years ago

Remove unused nsIDOMHTML*Element interfaces

Categories

(Core :: DOM: Core & HTML, enhancement)

enhancement
Not set

Tracking

()

RESOLVED FIXED
mozilla57
Tracking Status
firefox57 --- fixed

People

(Reporter: emk, Assigned: emk)

References

(Blocks 2 open bugs)

Details

Attachments

(2 files)

These interfaces are not QI'ed, so it is relatively easier to remove them.
Assignee: nobody → VYV03354
Status: NEW → ASSIGNED
Attachment #8897050 - Flags: review?(kyle)
Cancelling review because this patch caused many test failures.
Comment on attachment 8897050 [details]
Bug 1390209 - Remove unused nsIDOMHTML*Element interfaces.

https://reviewboard.mozilla.org/r/168334/#review176090

::: dom/interfaces/html/nsIDOMHTMLDocument.idl
(Diff revision 2)
>  interface nsIDOMHTMLDocument : nsIDOMDocument
>  {
>             attribute DOMString            domain;
>             attribute DOMString            cookie;
>  
> -  readonly attribute nsIDOMHTMLHeadElement head;

This is the only change I see in here that I've got a problem with. So far, we're removing interfaces as a whole and I'd like to keep it that way if possible. While I realize developers aren't supposed to be using these interfaces now and shouldn't expect them to work anyways, I'd rather take out interfaces all at once rather than in a piecemeal way, especially when they refer to the HTML spec. Otherwise I'm afraid we might get bugs about half-broken interfaces on our way to removing them all the way.
Attachment #8897050 - Flags: review?(kyle) → review-
Comment on attachment 8897050 [details]
Bug 1390209 - Remove unused nsIDOMHTML*Element interfaces.

https://reviewboard.mozilla.org/r/168334/#review176090

> This is the only change I see in here that I've got a problem with. So far, we're removing interfaces as a whole and I'd like to keep it that way if possible. While I realize developers aren't supposed to be using these interfaces now and shouldn't expect them to work anyways, I'd rather take out interfaces all at once rather than in a piecemeal way, especially when they refer to the HTML spec. Otherwise I'm afraid we might get bugs about half-broken interfaces on our way to removing them all the way.

I added back nsIDOMHTMLDocument.head using nsISupports. We have some precedents of using nsISupports when we migrated some interfaces to WebIDL.
Comment on attachment 8897050 [details]
Bug 1390209 - Remove unused nsIDOMHTML*Element interfaces.

https://reviewboard.mozilla.org/r/168334/#review176432
Attachment #8897050 - Flags: review?(kyle) → review+
Ok, well, thanks to mozreview I now can't un-r+ this, but I was looking up conflicts for comm-central and it looks like we're missing a few things in this patch. For instance, in mobile/android/chrome/content/browser.js, we still reference Ci.nsIDOMHTMLMenuElement. We need to go through and fix up these kinds of checks in chrome JS before we can land this.
Flags: needinfo?(VYV03354)
Went through all the interfaces we're taking out, and the thing I pointed out in Comment 8 looks like the only thing that was missed. So if you just want to attach an extra patch to this bug with a fix for that, or fix it in the current patch and carry the r+, I'll just call it an "r+ with nits".
Thanks for pointing out! Even if I updated the MozReview request, MozReview will not re-request the r+'ed review. So I a attached the change as an extra patch.
Flags: needinfo?(VYV03354)
Comment on attachment 8899997 [details]
Bug 1390209 - Follow-up patch to remove a reference to removed interfaces.

https://reviewboard.mozilla.org/r/171316/#review176510
Attachment #8899997 - Flags: review?(kyle) → review+
We're sorry, Autoland could not rebase your commits for you automatically. Please manually rebase your commits and try again.

hg error in cmd: hg rebase -s 217a1a28f80a -d 7f2356284595: rebasing 415135:217a1a28f80a "Bug 1390209 - Remove unused nsIDOMHTML*Element interfaces. r=qdot"
merging dom/html/HTMLStyleElement.h
warning: conflicts while merging dom/html/HTMLStyleElement.h! (edit, then use 'hg resolve --mark')
unresolved conflicts (see hg resolve, then hg rebase --continue)
We're sorry, Autoland could not rebase your commits for you automatically. Please manually rebase your commits and try again.

hg error in cmd: hg rebase -s 64bfa1e278c6 -d b76db9ba7cf6: rebasing 415136:64bfa1e278c6 "Bug 1390209 - Remove unused nsIDOMHTML*Element interfaces. r=qdot"
merging dom/html/HTMLStyleElement.h
warning: conflicts while merging dom/html/HTMLStyleElement.h! (edit, then use 'hg resolve --mark')
unresolved conflicts (see hg resolve, then hg rebase --continue)
Pushed by VYV03354@nifty.ne.jp:
https://hg.mozilla.org/integration/autoland/rev/6aa160333132
Remove unused nsIDOMHTML*Element interfaces. r=qdot
https://hg.mozilla.org/integration/autoland/rev/9a9a28c1778e
Follow-up patch to remove a reference to removed interfaces. r=qdot
https://hg.mozilla.org/mozilla-central/rev/6aa160333132
https://hg.mozilla.org/mozilla-central/rev/9a9a28c1778e
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
You need to log in before you can comment on or make changes to this bug.