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

Status: RESOLVED WORKSFORME

(Core :: DOM: Editor, defect)

Description

[Aryeh Gregor (:ayg) (no longer with Mozilla)]

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).

Comment 1

[Aryeh Gregor (:ayg) (no longer with Mozilla)]

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.