Remove nsIDOMHTMLAnchorElement type checks

RESOLVED FIXED in seamonkey2.58

Status

enhancement
RESOLVED FIXED
2 years ago
Last year

People

(Reporter: qdot, Assigned: frg)

Tracking

SeaMonkey 2.57 Branch
seamonkey2.58

SeaMonkey Tracking Flags

(seamonkey2.57esr fixed, seamonkey2.58 fixed)

Details

Attachments

(1 attachment, 3 obsolete attachments)

See https://bugzilla.mozilla.org/show_bug.cgi?id=1389650#c15. ChromeUtils.getClassName can be used to replace checks.
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → DUPLICATE
Duplicate of bug: 1403516
Actually, we didn't fix those bits in editor/ui/composer/content/editor.js.
Status: RESOLVED → UNCONFIRMED
Ever confirmed: false
Resolution: DUPLICATE → ---
Status: UNCONFIRMED → NEW
Ever confirmed: true
Posted patch 1403067.patch (obsolete) — Splinter Review
Also remove some more classes per bug 1403516 comment 33.

Untested as it is in Seamonkey.
Attachment #8914194 - Flags: review?(frgrahl)
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.
Attachment #8914194 - Flags: review-
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
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 on attachment 8914194 [details] [diff] [review]
1403067.patch

Clearing review for now. Aceman if you want I can do the fixup.
Flags: needinfo?(acelists)
Attachment #8914194 - Flags: review?(frgrahl)
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?
Flags: needinfo?(acelists)
Looks to be global: mozilla/dom/base/ChromeUtils.h
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)'.
Posted patch 1403067.patch v2 (obsolete) — Splinter Review
OK, so if you can test this in Seamonkey and finish what needs to be done on the node, that would be great. Thanks.
Attachment #8914194 - Attachment is obsolete: true
Attachment #8918671 - Flags: feedback?(frgrahl)
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.
Attachment #8918671 - Flags: feedback?(frgrahl) → feedback+
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?
Posted patch 1403067-wip.patch (obsolete) — Splinter Review
just rebased for testing
Attachment #8918671 - Attachment is obsolete: true
Assignee: nobody → frgrahl
Status: NEW → ASSIGNED
Version: unspecified → SeaMonkey 2.57 Branch
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: --
Attachment #8923127 - Attachment is obsolete: true
Attachment #8966026 - Flags: review?(iann_bugzilla)
Attachment #8966026 - Flags: approval-comm-beta?
Comment on attachment 8966026 [details] [diff] [review]
1403067-HTMLAnchorElement.patch

LGTM r/a=me
Attachment #8966026 - Flags: review?(iann_bugzilla)
Attachment #8966026 - Flags: review+
Attachment #8966026 - Flags: approval-comm-beta?
Attachment #8966026 - Flags: approval-comm-beta+
See Also: → 1452486
Pushed by frgrahl@gmx.net:
https://hg.mozilla.org/comm-central/rev/3a5516e5e969
Remove usage of nsIDOMHTMLAnchorElement in SeaMonkey. r=IanN
Status: ASSIGNED → RESOLVED
Closed: 2 years agoLast year
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.