Closed Bug 1088182 Opened 10 years ago Closed 9 years ago

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

Categories

(Core :: DOM: Editor, defect)

defect
Not set
minor

Tracking

()

RESOLVED WORKSFORME

People

(Reporter: ayg, Unassigned)

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
Closed: 9 years ago
Resolution: --- → WORKSFORME
You need to log in before you can comment on or make changes to this bug.