1403067 - Remove nsIDOMHTMLAnchorElement type checks

Status: RESOLVED FIXED

(SeaMonkey :: General, enhancement)

Description

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

See https://bugzilla.mozilla.org/show_bug.cgi?id=1389650#c15. ChromeUtils.getClassName can be used to replace checks.

Comment 2

[Jorg K (CEST = GMT+2)]

Actually, we didn't fix those bits in editor/ui/composer/content/editor.js.

Comment 3

[:aceman]

Attachment: 1403067.patch

Also remove some more classes per bug 1403516 comment 33.

Untested as it is in Seamonkey.

Comment 4

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

Comment on attachment 8914194 [details] [diff] [review]
1403067.patch

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

That method isn't going to work for those checks. You don't have access to those constructors in chrome JS. That's why I recommended the ChromeUtils.getClassName method in the bug description. There's examples of how this works in https://reviewboard.mozilla.org/r/167774/diff/6#index_header.

Comment 5

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

For further reference, here's the thread where we discussed the check methods on dev.platform: https://groups.google.com/forum/#!topic/mozilla.dev.platform/AGI46bcX1Qo

Comment 6

[Frank-Rainer Grahl (:frg)]

There is also Bug 1389622 Comment 4. Just haven't found the time yet to fix this one. 

ChromeUtils.getClassName(pluginElement) === "HTMLAnchorElement"

looks cleaner than the solution proposed in Bug 1389622.

Comment 7

[Frank-Rainer Grahl (:frg)]

Comment on attachment 8914194 [details] [diff] [review]
1403067.patch

Clearing review for now. Aceman if you want I can do the fixup.

Comment 8

[:aceman]

Yes, I can do that.
But where does ChromeUtils come from? I don't see it imported anywhere in the users. Is it some global object?

Comment 9

[Frank-Rainer Grahl (:frg)]

Looks to be global: mozilla/dom/base/ChromeUtils.h

Comment 10

[:aceman]

But when you have code:
if (node instanceof Components.interfaces.nsIDOMHTMLInputElement && node.type == "file")

and change it to:
if (ChromeUtils.getClassName(node) === "HTMLInputElement") then I'm not sure you can then use the .type attribute on node as the instanceof was doing QueryInterface on the node and now node will not have those properties. I think this can be changed to node.getAttribute("type") == "file", but I'm not sure what to do with cases like 'element.mozIsTextField(false)'.

Comment 11

[:aceman]

Attachment: 1403067.patch v2

OK, so if you can test this in Seamonkey and finish what needs to be done on the node, that would be great. Thanks.

Comment 12

[Frank-Rainer Grahl (:frg)]

Comment on attachment 8918671 [details] [diff] [review]
1403067.patch v2

aceman, big thanks. I have some small patches for other nsIDOM removal bugs kyle filed and will rebase and finish this one.

Comment 13

[Jorg K (CEST = GMT+2)]

Comment on attachment 8918671 [details] [diff] [review]
1403067.patch v2

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

::: editor/ui/composer/content/editor.js
@@ +3168,5 @@
>        }
>      }
>    } else {
>      for (node = document.tooltipNode; node; node = node.parentNode) {
> +      if (ChromeUtils.getClassName(node) === "HTMLImageElement" ||

What's wrong with .nodeName?

Comment 14

[Frank-Rainer Grahl (:frg)]

Attachment: 1403067-wip.patch

just rebased for testing

Comment 15

[Frank-Rainer Grahl (:frg)]

Attachment: 1403067-HTMLAnchorElement.patch

I am moving the nsIDOMHTMLInputElement to a new bug later.

[Approval Request Comment]
Regression caused by (bug #): Bug 1389650
User impact if declined: broken find
Testing completed (on m-c, etc.): c-b
Risk to taking this patch (and alternatives if risky): trivial
String changes made by this patch: --

Comment 16

[Ian Neal]

Comment on attachment 8966026 [details] [diff] [review]
1403067-HTMLAnchorElement.patch

LGTM r/a=me

Comment 17

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/3a5516e5e969
Remove usage of nsIDOMHTMLAnchorElement in SeaMonkey. r=IanN

Comment 18

[Frank-Rainer Grahl (:frg)]

https://hg.mozilla.org/releases/comm-beta/rev/6394caf01a8f7b7aa2b755149a9e9a92e8207b78