Issue 183893018: Check VisualPosition.isNull() after DOM mutation in ReplaceSelectionCommand

Issue 183893018: Check VisualPosition.isNull() after DOM mutation in ReplaceSelectionCommand (Closed)

Created:
6 years, 9 months ago by yoichio

Modified:
6 years, 9 months ago

Reviewers:
Yuta Kitamura

CC:
blink-reviews

Base URL:
https://chromium.googlesource.com/chromium/blink.git@master

Visibility:
Public.

More Reviews

Description

Check VisualPosition.isNull() after the DOM mutation in ReplaceSelectionCommand The crash is on L1125, calling insertNodeAt with the Null VisualPosition, startOfInsertedContent. It inserts a <br> element to the selected position but the replacing might change visual position so we should check. BUG=348283 Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=168502

Patch Set 1 #

Total comments: 6

Patch Set 2 : Update #

Created: 6 years, 9 months ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+38 lines, -5 lines)			Patch
A +	LayoutTests/editing/execCommand/insert-image-changing-visibility-crash.html	View		1 chunk	+3 lines, -3 lines	0 comments	Download
A +	LayoutTests/editing/execCommand/insert-image-changing-visibility-crash-expected.txt	View		1 chunk	+1 line, -1 line	0 comments	Download
A	LayoutTests/editing/execCommand/resources/insert-image-changing-visibility-crash-iframe.html	View	1	1 chunk	+33 lines, -0 lines	0 comments	Download
M	Source/core/editing/ReplaceSelectionCommand.cpp	View		1 chunk	+1 line, -1 line	0 comments	Download

Messages

Total messages: 6 (0 generated)

Expand Messages | Collapse Messages

Yuta Kitamura

Only nits, lgtm. https://codereview.chromium.org/183893018/diff/1/LayoutTests/editing/execCommand/resources/insert-image-changing-visibility-crash-iframe.html File LayoutTests/editing/execCommand/resources/insert-image-changing-visibility-crash-iframe.html (right): https://codereview.chromium.org/183893018/diff/1/LayoutTests/editing/execCommand/resources/insert-image-changing-visibility-crash-iframe.html#newcode11 LayoutTests/editing/execCommand/resources/insert-image-changing-visibility-crash-iframe.html:11: table.insertAdjacentHTML('afterbegin', "<svg/><div><div id=\"div\">text</div>"); nit: Don't use ...

6 years, 9 months ago (2014-03-04 06:36:18 UTC) #2

yoichio

https://codereview.chromium.org/183893018/diff/1/LayoutTests/editing/execCommand/resources/insert-image-changing-visibility-crash-iframe.html File LayoutTests/editing/execCommand/resources/insert-image-changing-visibility-crash-iframe.html (right): https://codereview.chromium.org/183893018/diff/1/LayoutTests/editing/execCommand/resources/insert-image-changing-visibility-crash-iframe.html#newcode11 LayoutTests/editing/execCommand/resources/insert-image-changing-visibility-crash-iframe.html:11: table.insertAdjacentHTML('afterbegin', "<svg/><div><div id=\"div\">text</div>"); On 2014/03/04 06:36:18, Yuta Kitamura wrote: ...

6 years, 9 months ago (2014-03-04 07:05:55 UTC) #3

commit-bot: I haz the power

CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/yoichio@chromium.org/183893018/2

6 years, 9 months ago (2014-03-05 00:54:22 UTC) #5

Message was sent while issue was closed.

Change committed as 168502

Expand Messages | Collapse Messages