Bug 41284 – ApplyStyleCommand::splitTextElementAtEndIfNeeded/splitTextAtEndIfNeeded ASSERTS should be returns.

• Component: HTML Editing
• Status: RESOLVED INVALID

[Mike Fenton]

Created attachment 59909 [details]
Patch

Both splitTextElementAtStartIfNeeded and splitTextAtEndIfNeeded call splitTextNodeContainingElement and splitTextNode respectively assuming that the split will succeed and verify the resulting using ASSERTS.

In both functions there are a number of early returns possibly.  The state after the split occurs should be validated with an if statement so that a proper return can happen.

[Ojan Vafai]

Comment on attachment 59909 [details]
Patch

Why is asserting wrong? When do we hit this? Does this cause a change in behavior? If so, can you add a layout test?

[Ryosuke Niwa]

(In reply to comment #0)
> In both functions there are a number of early returns possibly.  The state after the split occurs should be validated with an if statement so that a proper return can happen.

Let me explain to you why this doesn't happen.  For splitTextAtEndIfNeeded, splitTextNode will create a text node before the text.  So, text->previousSibling() should ALWAYS return a valid node.  Since splitTextNode does not delete or move any nodes, we know that ALL start.node(), end.node(), and prevNode are valid pointers.

For splitTextElementAtEndIfNeeded, splitTextNodeContainingElement splits the text node along with its container.  So if we have <elem><text></elem>, we get <elem#1><text#1></elem#1><elem#2><text#2></elem#2> where <text#1><text#2>=<text>.  Now, text is pointing at <text#2> at this point because splitTextNodeContainingElement adds <text#1> and strips a part of text from <text> to make <text#2> (i.e. <text> and <text#2> share the same pointer value).  Now, text->parent()->previousSibling() is pointing at <elem#1>.  The last child of <elem#1> is <text#1>, and this node should exist as long as splitTextNodeContainingElement didn't blow up.  I omit the argument for startNode in this case because it's identical to that of splitTextAtEndIfNeeded.