Summary: | Nullptr dereference in DeleteSelectionCommand::mergeParagraphs() | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Ian Gilbert <iang> | ||||||||
Component: | HTML Editing | Assignee: | Nobody <webkit-unassigned> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | bfulgham, cgarcia, ews-feeder, product-security, rbuis, rniwa, svillar, webkit-bug-importer, wenson_hsieh | ||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||
Version: | WebKit Local Build | ||||||||||
Hardware: | Unspecified | ||||||||||
OS: | Unspecified | ||||||||||
Attachments: |
|
Description
Ian Gilbert
2020-11-03 02:20:27 PST
Created attachment 413025 [details]
Crashing input
Something like this is supposed to work but there are some test failures: Index: Source/WebCore/editing/CompositeEditCommand.cpp =================================================================== --- Source/WebCore/editing/CompositeEditCommand.cpp (revision 268963) +++ Source/WebCore/editing/CompositeEditCommand.cpp (working copy) @@ -1456,8 +1456,9 @@ ASSERT(destination.deepEquivalent().anchorNode()->isConnected()); cleanupAfterDeletion(destination); + auto newEndingSelection = VisibleSelection(destination, originalIsDirectional); // FIXME (Bug 211793): We should redesign cleanupAfterDeletion or find another destination when it is removed. - if (!destination.deepEquivalent().anchorNode()->isConnected()) + if (!destination.deepEquivalent().anchorNode()->isConnected() || newEndingSelection.isNone()) return; // Add a br if pruning an empty block level element caused a collapse. For example: @@ -1482,7 +1483,7 @@ auto destinationIndex = characterCount({ { *editableRoot, 0 }, *makeBoundaryPoint(destination) }, TextIteratorEmitsCharactersBetweenAllVisiblePositions); - setEndingSelection(VisibleSelection(destination, originalIsDirectional)); + setEndingSelection(newEndingSelection); ASSERT(endingSelection().isCaretOrRange()); OptionSet<ReplaceSelectionCommand::CommandOption> options { ReplaceSelectionCommand::SelectReplacement, ReplaceSelectionCommand::MovingParagraph }; if (!preserveStyle) Created attachment 414166 [details]
Patch
Created attachment 414212 [details]
Patch
Comment on attachment 414212 [details]
Patch
There is probably some overlap with 218492.
(In reply to Rob Buis from comment #7) > Comment on attachment 414212 [details] > Patch > > There is probably some overlap with 218492. In that case, can we re-land the fix for the bug 218492 first and check if it also fixes this bug or not? (In reply to Ryosuke Niwa from comment #8) > (In reply to Rob Buis from comment #7) > > Comment on attachment 414212 [details] > > Patch > > > > There is probably some overlap with 218492. > > In that case, can we re-land the fix for the bug 218492 first and check if > it also fixes this bug or not? I *think* this is happening (sorry my wording was a bit imprecise): - 218492 bt is specific to Mac. GTK instead has the bt from 218491 for the test case in 218492. - so the 218492 fix for Mac had a test case that fixes 218492 on Mac and thus equals it to GTK status, i.e. still hitting 218491. Hope that clears things up. If I am right fixing 218491 first and then relanding the 218492 fix should fix both bugs. Ah ok. Committed r269894: <https://trac.webkit.org/changeset/269894> All reviewed patches have been landed. Closing bug and clearing flags on attachment 414212 [details]. |