Summary: | REGRESSION: Pasting text from TextEdit with a bold word into text field results in crash | ||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Mark Rowe (bdash) <mrowe> | ||||||||||||
Component: | HTML Editing | Assignee: | Justin Garcia <justin.garcia> | ||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||
Severity: | Normal | CC: | c.petersen87, darin, justin.garcia | ||||||||||||
Priority: | P1 | Keywords: | InRadar, Regression | ||||||||||||
Version: | 420+ | ||||||||||||||
Hardware: | Mac | ||||||||||||||
OS: | OS X 10.4 | ||||||||||||||
Attachments: |
|
Description
Mark Rowe (bdash)
2006-04-02 15:20:41 PDT
Created attachment 7470 [details]
Crash log.
Crash happens because a node is nil. This happens because of this code in ReplaceSelectionCommand::doApply: 702 } else if (m_lastNodeInserted && !m_fragment.isBlockFlow(refNode)) { 703 Position pos = visiblePos.next().deepEquivalent().downstream(); 704 insertNodeAtAndUpdateNodesInserted(refNode, pos.node(), pos.offset()); visiblePos.next() is null. Created attachment 7471 [details]
Rough patch
The patch simply adds a NULL check before the call to insertNodeAtAndUpdateNodesInserted. I don't really understand the code in question so I am in no way confident that it is the right fix, however it prevents the crash and doesn't appear to change any other behaviour.
Comment on attachment 7471 [details]
Rough patch
I believe this is wrong and will result in the pasted text not being inserted at all!
The right thing is to find a way to insert after visiblePos instead of before visiblePos.next() in this case.
Justin should probably handle this one himself. The pasted text is definitely inserted correctly with that patch, but I'm more than willing to let Justin handle this. These are all text field regressions so they should all be P1. *** Bug 8149 has been marked as a duplicate of this bug. *** Same as <rdar://problem/4499431>. Created attachment 7545 [details]
patch, changelog and layout tests
Created attachment 7546 [details]
patch, changelog and layout tests
Comment on attachment 7546 [details]
patch, changelog and layout tests
dave reviewed this in person, and he's going to look it over and do a final review tonight.
Created attachment 7552 [details]
patch, changelog and layout tests
One more revision of my changelog and code comments.
Comment on attachment 7552 [details]
patch, changelog and layout tests
dave reviewed this
|