Drop "namedanchor" support from nsIHTMLEditor::CreateElementWithDefaults(), nsIHTMLEditor::GetSelectedElement() and nsIHTMLEditor::GetElementOrParentByTagName()

RESOLVED FIXED in Firefox 63

Status

()

enhancement
RESOLVED FIXED
11 months ago
10 months ago

People

(Reporter: masayuki, Assigned: masayuki)

Tracking

({dev-doc-complete})

Trunk
mozilla63
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox63 fixed)

Details

Attachments

(3 attachments)

nsIHTMLEditor::CreateElementWithDefaults() and nsIHTMLEditor::GetSelectedElement() treats "namedanchor" as <a name="..."> element. However, it seems that this is legacy name since there is another name, "anchor", and only "anchor" is used in comm-central and BlueGriffon. Due to not defined as an Atom, it does not make sense to keep supporting this value.
nsIHTMLEditor::GetElementOrParentByTagName() is the last user of IsNamedAnchorTag(const nsAString&) and this is not used with "namedanchor" actually. So, we can completely drop "namedanchor".
Summary: Drop "namedanchor" support from nsIHTMLEditor::CreateElementWithDefaults() and nsIHTMLEditor::GetSelectedElement() → Drop "namedanchor" support from nsIHTMLEditor::CreateElementWithDefaults(), nsIHTMLEditor::GetSelectedElement() and nsIHTMLEditor::GetElementOrParentByTagName()
HTMLElementOrParentByTagName() is the last user of IsLinkTag(const nsAString&)
and IsNamedAnchorTag(const nsAString&).  For making their maintenance easier,
let's make GetElementOrParentByTagName() take const nsAtom& for tag name.

GetElementOrParentByTagName() has two functions, one is looking for an element
starting from a node.  The other is, if the start node is nullptr, it retrieves
anchor node of Selection as start node.  Therefore, this patch splits the
first part to GetElementOrParentByTagNameInternal().  Then, creates its
wrapper which retrieves anchor of Selection automatically,
GetElementOrParentByTagNameAtSelection().

Additionally, this patch makes all internal callers of HTMLEditor use
GetElementOrParentByTagNameInternal() or
GetElementOrParentByTagNameAtSelection() directly.  Then, public method,
GetElementOrParentByTagName() is called only by outer classes.

Note that some callers use both GetElementOrParentByTagNameInternal()
and GetElementOrParentByTagNameAtSelection() since they don't check whether
setting node is nullptr.  They may be bug of them.  We should investigate
the API callers later.
Nobody (including comm-central and BlueGriffon) does not use "namedanchor"
special element name with those XPCOMs.  Of course, our internal callers too.
Therefore, we can drop.

Note that there is no static Atom for this, so, keeping it makes unnecessary
runtime cost for Firefox users.

This could cause breaking some legacy add-ons for Thunderbird.  However,
they can use "anchor" special element name for same purpose.
The methods compared with const characters since we've supported "namedanchor"
which is not in nsGkAtoms.  Now, it's dropped so that we can compare given
atom with nsGkAtoms.
Comment on attachment 9001862 [details]
Bug 1484092 - part 1: Make HTMLEditor::GetElementOrParentByTagName() use nsAtom for the tag name

Makoto Kato [:m_kato] has approved the revision.
Attachment #9001862 - Flags: review+
Comment on attachment 9001863 [details]
Bug 1484092 - part 2: Drop supporting "namedanchor" special element name from nsIHTMLEditor::GetSelectedElement(), nsIHTMLEditor::GetElementOrParentByTagName() and nsIHTMLEditor::CreateElementWithDefaults()

Makoto Kato [:m_kato] has approved the revision.
Attachment #9001863 - Flags: review+
Comment on attachment 9001864 [details]
Bug 1484092 - part 3: IsLinkTag() and IsNamedAnchorTag() should compare with nsGkAtoms

Makoto Kato [:m_kato] has approved the revision.
Attachment #9001864 - Flags: review+
Pushed by masayuki@d-toybox.com:
https://hg.mozilla.org/integration/autoland/rev/d0b14e8711df
part 1: Make HTMLEditor::GetElementOrParentByTagName() use nsAtom for the tag name r=m_kato
Pushed by masayuki@d-toybox.com:
https://hg.mozilla.org/integration/autoland/rev/10fdd041f1b5
part 2: Drop supporting "namedanchor" special element name from nsIHTMLEditor::GetSelectedElement(), nsIHTMLEditor::GetElementOrParentByTagName() and nsIHTMLEditor::CreateElementWithDefaults() r=m_kato
Oh, the line will be overwritten by the next patch. Sorry for the mistake. And I really hope Lando support to land multiple patches once...
Flags: needinfo?(masayuki)
Pushed by masayuki@d-toybox.com:
https://hg.mozilla.org/integration/autoland/rev/717fb8632016
part 1: Make HTMLEditor::GetElementOrParentByTagName() use nsAtom for the tag name r=m_kato
Hmm... Lando failed to rebase at part 2 so that only part 1 landed first... Sigh...
Keywords: leave-open
Pushed by masayuki@d-toybox.com:
https://hg.mozilla.org/integration/autoland/rev/a2a95c8855db
part 2: Drop supporting "namedanchor" special element name from nsIHTMLEditor::GetSelectedElement(), nsIHTMLEditor::GetElementOrParentByTagName() and nsIHTMLEditor::CreateElementWithDefaults() r=m_kato
Pushed by masayuki@d-toybox.com:
https://hg.mozilla.org/integration/autoland/rev/4b5906a29b0c
part 3: IsLinkTag() and IsNamedAnchorTag() should compare with nsGkAtoms r=m_kato
Status: ASSIGNED → RESOLVED
Closed: 11 months ago
Keywords: leave-open
Resolution: --- → FIXED
Target Milestone: --- → mozilla63
Keywords: dev-doc-needed
I've updated the following page:

https://developer.mozilla.org/en-US/docs/Mozilla/Tech/XPCOM/Reference/Interface/nsIHTMLEditor

to remove mention of named anchors from the sections on CreateElementWithDefaults(), GetSelectedElement(), and GetElementOrParentByTagName(). Let me know if you need anything else.
You need to log in before you can comment on or make changes to this bug.