Closed Bug 1418076 Opened 2 years ago Closed 2 years ago

Remove nsIDOMHTMLDocument

Categories

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

enhancement

Tracking

()

RESOLVED FIXED
mozilla60
Tracking Status
firefox60 --- fixed

People

(Reporter: qdot, Assigned: bzbarsky)

References

(Blocks 1 open bug)

Details

Attachments

(11 files, 1 obsolete file)

3.62 KB, patch
Nika
: review+
Details | Diff | Splinter Review
2.78 KB, patch
Nika
: review+
Details | Diff | Splinter Review
1.92 KB, patch
Nika
: review+
Details | Diff | Splinter Review
10.75 KB, patch
Nika
: review+
Details | Diff | Splinter Review
9.24 KB, patch
Nika
: review+
Details | Diff | Splinter Review
3.85 KB, patch
Nika
: review+
Details | Diff | Splinter Review
5.30 KB, patch
Nika
: review+
Details | Diff | Splinter Review
3.39 KB, patch
Nika
: review+
Details | Diff | Splinter Review
4.11 KB, patch
Nika
: review+
Details | Diff | Splinter Review
16.12 KB, patch
Nika
: review+
Details | Diff | Splinter Review
8.34 KB, patch
Nika
: review+
Details | Diff | Splinter Review
Continuing post-addon-deprecation XPCOM interface cleanup
Priority: -- → P3
Patches coming up.
Depends on: 1276438
Assignee: kyle → bzbarsky
Status: NEW → ASSIGNED
Depends on: 1433363
MozReview-Commit-ID: 4E6skumHqNP
Attachment #8945850 - Flags: review?(nika)
Attachment #8945704 - Attachment is obsolete: true
Attachment #8945704 - Flags: review?(nika)
Attachment #8945695 - Flags: review?(nika) → review+
Attachment #8945696 - Flags: review?(nika) → review+
Attachment #8945697 - Flags: review?(nika) → review+
Attachment #8945698 - Flags: review?(nika) → review+
Comment on attachment 8945699 [details] [diff] [review]
part 5.  Get rid of nsIDOMHTMLDocument's designMode attribute

Review of attachment 8945699 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/base/nsFrameLoader.cpp
@@ +915,5 @@
> +          htmlDoc->SetDesignMode(NS_LITERAL_STRING("off"), Nothing(), rv);
> +        }
> +
> +        IgnoredErrorResult rv;
> +        htmlDoc->SetDesignMode(NS_LITERAL_STRING("on"), Nothing(), rv);

Can you not re-use IgnoredErrorResult values?
Attachment #8945699 - Flags: review?(nika) → review+
Attachment #8945700 - Flags: review?(nika) → review+
Attachment #8945701 - Flags: review?(nika) → review+
Comment on attachment 8945702 [details] [diff] [review]
part 8.  Remove remaining, unused, nsIDOMHTMLDocument methods

Review of attachment 8945702 [details] [diff] [review]:
-----------------------------------------------------------------

Feels good to see so many lines being deleted.
Attachment #8945702 - Flags: review?(nika) → review+
> Can you not re-use IgnoredErrorResult values?

If they were thrown on, not really.  They do their ignoring and cleanup in the destructor.
Attachment #8945703 - Flags: review?(nika) → review+
Comment on attachment 8945705 [details] [diff] [review]
part 11.  Eliminate the nsIDOMHTMLDocument interface

Review of attachment 8945705 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/html/nsHTMLDocument.cpp
@@ +1298,5 @@
>                       const nsAString& aFeatures,
>                       bool aReplace,
>                       ErrorResult& rv)
>  {
> +  NS_ASSERTION(nsContentUtils::CanCallerAccess(static_cast<nsIDOMDocument*>(this)),

Maybe turn these into asserts while we're here?

@@ +1328,5 @@
>  {
>    // Implements the "When called with two arguments (or fewer)" steps here:
>    // https://html.spec.whatwg.org/multipage/webappapis.html#opening-the-input-stream
>  
> +  NS_ASSERTION(nsContentUtils::CanCallerAccess(static_cast<nsIDOMDocument*>(this)),

Here too.
Attachment #8945705 - Flags: review?(nika) → review+
Comment on attachment 8945850 [details] [diff] [review]
part 10.  Get rid of JS uses of Ci.nsIDOMHTMLDocument

Review of attachment 8945850 [details] [diff] [review]:
-----------------------------------------------------------------

::: devtools/server/actors/styles.js
@@ +1013,5 @@
>    },
>  
>    getDocument: function (sheet) {
>      if (sheet.ownerNode) {
> +      return sheet.ownerNode.nodeType == sheet.ownerNode.DOCUMENT_NODE ?

Why don't you use ChromeUtils.getClassName(...) == "HTMLDocument" here and below?

Seems to me like it would make sense to be consistent here?
Attachment #8945850 - Flags: review?(nika) → review+
> Maybe turn these into asserts while we're here?

You mean MOZ_ASSERT?  Can do.

> Why don't you use ChromeUtils.getClassName(...) == "HTMLDocument" here and below?

Because non-HTML documents can have stylesheets.  That is, this is a purposeful behavior change to check for a Document (or any subclass thereof), not an HTMLDocument.

The other places where I left the getClassName thing seemed to really want HTML documents...
Pushed by bzbarsky@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/e2b9d1d7a5e9
part 1.  Get rid of nsIDOMHTMLDocument's domain attribute.  r=mystor
https://hg.mozilla.org/integration/mozilla-inbound/rev/ffbb81730650
part 2.  Get rid of nsIDOMHTMLDocument's cookie attribute.  r=mystor
https://hg.mozilla.org/integration/mozilla-inbound/rev/e0146922d6a1
part 3.  Get rid of nsIDOMHTMLDocument's head attribute.  r=mystor
https://hg.mozilla.org/integration/mozilla-inbound/rev/d57a1d68c0d8
part 4.  Get rid of nsIDOMHTMLDocument's open/close/write/writeln methods.  r=mystor
https://hg.mozilla.org/integration/mozilla-inbound/rev/1f42b3511868
part 5.  Get rid of nsIDOMHTMLDocument's designMode attribute.  r=mystor
https://hg.mozilla.org/integration/mozilla-inbound/rev/d83172c57cab
part 6.  Get rid of nsIDOMHTMLDocument's queryCommand* methods.  r=mystor
https://hg.mozilla.org/integration/mozilla-inbound/rev/271000186433
part 7.  Get rid of nsIDOMHTMLDocument's color attributes.  r=mystor
https://hg.mozilla.org/integration/mozilla-inbound/rev/25a8a0f535be
part 8.  Remove remaining, unused, nsIDOMHTMLDocument methods.  r=mystor.
https://hg.mozilla.org/integration/mozilla-inbound/rev/60a9954c827b
part 9.  Eliminate remaining uses of nsIDOMHTMLDocument.  r=mystor
https://hg.mozilla.org/integration/mozilla-inbound/rev/6cfd2108d5ce
part 10.  Get rid of JS uses of Ci.nsIDOMHTMLDocument.  r=mystor
https://hg.mozilla.org/integration/mozilla-inbound/rev/7a62766df44f
part 11.  Eliminate the nsIDOMHTMLDocument interface.  r=mystor
You need to log in before you can comment on or make changes to this bug.