1418085 - Remove nsIDOMHTMLElement

Status: RESOLVED FIXED

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

Description

[Kyle Machulis [:qdot] [:kmachulis] (INACTIVE)]

Continuing post-addon-deprecation XPCOM interface cleanup

Comment 1

[Boris Zbarsky [:bzbarsky]]

Attachment: part 1. Stop using nsIDOMHTMLElement in editor code

MozReview-Commit-ID: 6J0wWzMCfWP

Comment 2

[Boris Zbarsky [:bzbarsky]]

Attachment: part 2. Stop using nsIDOMHTMLElement in accessibility code

MozReview-Commit-ID: 6YddkxqB5Bv

Comment 3

[Boris Zbarsky [:bzbarsky]]

Attachment: part 3. Stop using nsIDOMHTMLElement in form fill

MozReview-Commit-ID: Ax7RUZQCosr

Comment 4

[Boris Zbarsky [:bzbarsky]]

Attachment: part 4. Stop using nsIDOMHTMLElement in JS code

MozReview-Commit-ID: GsSnXNXGrHg

Comment 5

[Boris Zbarsky [:bzbarsky]]

Attachment: part 5. Remove nsIDOMHTMLInputElement::GetList

MozReview-Commit-ID: JbWMti82q3R

Comment 6

[Boris Zbarsky [:bzbarsky]]

Attachment: part 6. Remove nsIDOMHTMLElement

MozReview-Commit-ID: 5QUyFeAQYZQ

Comment 7

[Boris Zbarsky [:bzbarsky]]

Attachment: part 2. Stop using nsIDOMHTMLElement in accessibility code

MozReview-Commit-ID: 6YddkxqB5Bv

Comment 8

[alexander :surkov (:asurkov)]

Comment on attachment 8945964 [details] [diff] [review]
part 2.  Stop using nsIDOMHTMLElement in accessibility code

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

::: accessible/tests/mochitest/events.js
@@ +1059,5 @@
>      }
>  
>      // Scroll the node into view, otherwise synth click may fail.
> +    if (isHTMLElement(targetNode)) {
> +      // XXXbz scrollIntoView is on Element; can we just use it unconditionally?

yes, we should be able

@@ +1068,5 @@
>      }
>  
>      var x = 1, y = 1;
>      if (aArgs && ("where" in aArgs) && aArgs.where == "right") {
> +      if (isHTMLElement(targetNode))

getBoundingClientRect() probably can help to get rid the check too

Comment 9

[Nika Layzell [:nika] (ni? for response)]

Comment on attachment 8945925 [details] [diff] [review]
part 1.  Stop using nsIDOMHTMLElement in editor code

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

::: editor/libeditor/HTMLEditor.cpp
@@ +353,5 @@
>  {
>    // Use the HTML documents body element as the editor root if we didn't
>    // get a root element during initialization.
>  
> +  Element* bodyElement = GetBodyElement();

Why not do:

mRootElement = GetBodyElement();
if (!mRootElement) {
  nsCOMPtr<nsIDocument> doc = GetDocument();
  if (doc) {
    mRootElement = doc->GetDocumentElement();
  }
}

@@ +368,2 @@
>      }
> +  }    

nit: trailing whitespace.

Comment 10

[Boris Zbarsky [:bzbarsky]]

> Why not do:

Because I didn't think to.  ;)  Fixed.

> nit: trailing whitespace.

Fixed.

Comment 11

[Nika Layzell [:nika] (ni? for response)]

Comment on attachment 8945929 [details] [diff] [review]
part 4.  Stop using nsIDOMHTMLElement in JS code

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

::: devtools/server/actors/object.js
@@ +1853,5 @@
>          }
>        }
>      } else if (rawObj instanceof Ci.nsIDOMElement) {
>        // Add preview for DOM element attributes.
> +      if (rawObj.namespaceURI == "http://www.w3.org/1999/xhtml") {

I would like an explanation as to why this change is correct - it doesn't seem necessarily incorrect but it also isn't super clear as to what you're checking.

Comment 12

[Nika Layzell [:nika] (ni? for response)]

Comment on attachment 8945931 [details] [diff] [review]
part 5.  Remove nsIDOMHTMLInputElement::GetList

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

::: dom/interfaces/html/nsIDOMHTMLInputElement.idl
@@ +34,5 @@
>             attribute DOMString             name;
>  
>             attribute boolean               readOnly;
>  
>    readonly attribute nsIControllers        controllers;	

Maybe clean up this whitespace while you're here?

Comment 13

[Boris Zbarsky [:bzbarsky]]

Attachment: Part 4 with comments explaining what it's doing

Comment 14

[Boris Zbarsky [:bzbarsky]]

> Maybe clean up this whitespace while you're here?

Done.

Comment 15

[Boris Zbarsky [:bzbarsky]]

Attachment: Part 4 with comments explaining what it's doing

Comment 16

[Nika Layzell [:nika] (ni? for response)]

Comment on attachment 8946349 [details] [diff] [review]
Part 4 with comments explaining what it's doing

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

thanks

Comment 17

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/2c852fd27256
part 1.  Stop using nsIDOMHTMLElement in editor code.  r=mystor
https://hg.mozilla.org/integration/mozilla-inbound/rev/09cb8eececa7
part 2.  Stop using nsIDOMHTMLElement in accessibility code.  r=surkov
https://hg.mozilla.org/integration/mozilla-inbound/rev/0ff82b4a39e1
part 3.  Stop using nsIDOMHTMLElement in form fill.  r=mystor
https://hg.mozilla.org/integration/mozilla-inbound/rev/8f73def3b816
part 4.  Stop using nsIDOMHTMLElement in JS code.  r=mystor
https://hg.mozilla.org/integration/mozilla-inbound/rev/86ea8b042827
part 5.  Remove nsIDOMHTMLInputElement::GetList.  r=mystor
https://hg.mozilla.org/integration/mozilla-inbound/rev/09e8b8497618
part 6.  Remove nsIDOMHTMLElement.  r=mystor

Comment 18

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/6e2d37b1b955
followup.  Try to fix the Mac-only a11y orange.  r=bustage

Comment 19

[Boris Zbarsky [:bzbarsky]]

Either scrollIntoView or getBoundingClientRect didn't do the right thing on Mac....

Comment 20

[Raul Gurzau (:RaulG)]

https://hg.mozilla.org/mozilla-central/rev/2c852fd27256
https://hg.mozilla.org/mozilla-central/rev/09cb8eececa7
https://hg.mozilla.org/mozilla-central/rev/0ff82b4a39e1
https://hg.mozilla.org/mozilla-central/rev/8f73def3b816
https://hg.mozilla.org/mozilla-central/rev/86ea8b042827
https://hg.mozilla.org/mozilla-central/rev/09e8b8497618
https://hg.mozilla.org/mozilla-central/rev/6e2d37b1b955