Audit methods like IsBreak for differences between nsINode* and nsIDOMNode* variants

RESOLVED WORKSFORME

Status

()

--
minor
RESOLVED WORKSFORME
4 years ago
4 years ago

People

(Reporter: ayg, Unassigned)

Tracking

Trunk
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

I noticed nsTextEditUtils::IsBreak(nsIDOMNode*) just checks that the tag is br, while nsTextEditUtils::IsBreak(nsINode*) uses IsHTML(), which checks slightly more.  In this exact case I'm not worried that it will break anything, but in general, nsINode*/nsIDOMNode* variants need to behave identically (preferably one wrapping the other) to avoid risk of breaking during rewrites.  In this case, the nsINode* variant should be removed and callers should be replaced by checking the tag (in line with the nsIDOMNode* variant).
In this specific case, Tag() has gone away, so the sensible thing to do in this cases is to use IsHTMLElement() anyway.  I guess we can be pretty sure it won't break anything.  I briefly looked over all the stuff in nsTextEditUtils and nsHTMLEditUtils and didn't find any discrepancies other than that, and differences in null-checking (the nsINode ones tend to assert or crash instead of returning false).  Not sure how much to worry about that -- it can cause regressions, but it's generally better behavior.
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → WORKSFORME
You need to log in before you can comment on or make changes to this bug.