Skip to content

Commit 86a313a

Browse files
whsiehwhsieh
committed
DeleteSelectionCommand should be robust when starting and ending editable positions cannot be found
https://bugs.webkit.org/show_bug.cgi?id=175914 <rdar://problem/29792688> Reviewed by Ryosuke Niwa. Source/WebCore: DeleteSelectionCommand can cause a null dereference if editable start and end positions are not found. This can happen when attempting to delete after selecting the contents within a canvas or output element with `read-write` `-webkit-user-modify` style. To fix this, we make the initialization step of the DeleteSelectionCommand robust when editable start and end positions are missing. Test: editing/execCommand/forward-delete-read-write-canvas.html * editing/DeleteSelectionCommand.cpp: (WebCore::DeleteSelectionCommand::initializePositionData): Make this initialization helper indicate failure via a bool return value. DeleteSelectionCommand::doApply bails early if initializePositionData returned false. (WebCore::DeleteSelectionCommand::doApply): * editing/DeleteSelectionCommand.h: LayoutTests: Adds a new LayoutTest. This test passes if WebKit successfully loaded the page. * editing/execCommand/forward-delete-read-write-canvas-expected.txt: Added. * editing/execCommand/forward-delete-read-write-canvas.html: Added. Canonical link: https://commits.webkit.org/192580@main git-svn-id: https://svn.webkit.org/repository/webkit/trunk@221128 268f45cc-cd09-0410-ab3c-d52691b4dbfc
1 parent 8f4a575 commit 86a313a

6 files changed

Lines changed: 56 additions & 3 deletions

File tree

‎LayoutTests/ChangeLog‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,16 @@
1+
2017-08-23 Wenson Hsieh <wenson_hsieh@apple.com>
2+
3+
DeleteSelectionCommand should be robust when starting and ending editable positions cannot be found
4+
https://bugs.webkit.org/show_bug.cgi?id=175914
5+
<rdar://problem/29792688>
6+
7+
Reviewed by Ryosuke Niwa.
8+
9+
Adds a new LayoutTest. This test passes if WebKit successfully loaded the page.
10+
11+
* editing/execCommand/forward-delete-read-write-canvas-expected.txt: Added.
12+
* editing/execCommand/forward-delete-read-write-canvas.html: Added.
13+
114
2017-08-23 Matt Lewis <jlewis3@apple.com>
215

316
Marked loader/stateobjects/replacestate-size.html as flaky.

‎LayoutTests/editing/execCommand/forward-delete-read-write-canvas-expected.txt‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
PASS

‎LayoutTests/editing/execCommand/forward-delete-read-write-canvas.html‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
<code style="color: green">PASS</code>
2+
<canvas style="-webkit-user-modify: read-write"><span><input id="input"></input></span></canvas>
3+
<script>
4+
if (window.testRunner)
5+
testRunner.dumpAsText();
6+
input.focus();
7+
input.remove();
8+
document.execCommand("ForwardDelete");
9+
</script>

‎Source/WebCore/ChangeLog‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,27 @@
1+
2017-08-23 Wenson Hsieh <wenson_hsieh@apple.com>
2+
3+
DeleteSelectionCommand should be robust when starting and ending editable positions cannot be found
4+
https://bugs.webkit.org/show_bug.cgi?id=175914
5+
<rdar://problem/29792688>
6+
7+
Reviewed by Ryosuke Niwa.
8+
9+
DeleteSelectionCommand can cause a null dereference if editable start and end positions are not found. This can
10+
happen when attempting to delete after selecting the contents within a canvas or output element with `read-write`
11+
`-webkit-user-modify` style. To fix this, we make the initialization step of the DeleteSelectionCommand robust
12+
when editable start and end positions are missing.
13+
14+
Test: editing/execCommand/forward-delete-read-write-canvas.html
15+
16+
* editing/DeleteSelectionCommand.cpp:
17+
(WebCore::DeleteSelectionCommand::initializePositionData):
18+
19+
Make this initialization helper indicate failure via a bool return value. DeleteSelectionCommand::doApply bails
20+
early if initializePositionData returned false.
21+
22+
(WebCore::DeleteSelectionCommand::doApply):
23+
* editing/DeleteSelectionCommand.h:
24+
125
2017-08-23 Youenn Fablet <youenn@apple.com>
226

327
[Cache API] Unify WebCore and WebKit error handling

‎Source/WebCore/editing/DeleteSelectionCommand.cpp‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -172,7 +172,7 @@ void DeleteSelectionCommand::setStartingSelectionOnSmartDelete(const Position& s
172172
setStartingSelection(VisibleSelection(newBase, newExtent, startingSelection().isDirectional()));
173173
}
174174

175-
void DeleteSelectionCommand::initializePositionData()
175+
bool DeleteSelectionCommand::initializePositionData()
176176
{
177177
Position start, end;
178178
initializeStartEnd(start, end);
@@ -182,6 +182,9 @@ void DeleteSelectionCommand::initializePositionData()
182182
if (!isEditablePosition(end, ContentIsEditable))
183183
end = lastEditablePositionBeforePositionInRoot(end, highestEditableRoot(start));
184184

185+
if (start.isNull() || end.isNull())
186+
return false;
187+
185188
m_upstreamStart = start.upstream();
186189
m_downstreamStart = start.downstream();
187190
m_upstreamEnd = end.upstream();
@@ -272,6 +275,8 @@ void DeleteSelectionCommand::initializePositionData()
272275
// node. This was done to match existing behavior, but it seems wrong.
273276
m_startBlock = enclosingNodeOfType(m_downstreamStart.parentAnchoredEquivalent(), &isBlock, CanCrossEditingBoundary);
274277
m_endBlock = enclosingNodeOfType(m_upstreamEnd.parentAnchoredEquivalent(), &isBlock, CanCrossEditingBoundary);
278+
279+
return true;
275280
}
276281

277282
void DeleteSelectionCommand::saveTypingStyleState()
@@ -857,7 +862,8 @@ void DeleteSelectionCommand::doApply()
857862

858863

859864
// set up our state
860-
initializePositionData();
865+
if (!initializePositionData())
866+
return;
861867

862868
// Delete any text that may hinder our ability to fixup whitespace after the delete
863869
deleteInsignificantTextDownstream(m_trailingWhitespace);

‎Source/WebCore/editing/DeleteSelectionCommand.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ class DeleteSelectionCommand : public CompositeEditCommand {
5454

5555
void initializeStartEnd(Position&, Position&);
5656
void setStartingSelectionOnSmartDelete(const Position&, const Position&);
57-
void initializePositionData();
57+
bool initializePositionData();
5858
void saveTypingStyleState();
5959
void insertPlaceholderForAncestorBlockContent();
6060
bool handleSpecialCaseBRDelete();

0 commit comments

Comments
 (0)