New issue
Advanced search Search tips

Issue 598665 link

Starred by 0 users

Issue metadata

Status: Fixed
Owner:
Closed: Apr 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows , All
Pri: 2
Type: Bug



Sign in to add a comment

InsertParagraphSeparatorCommand should handle whitespace:pre correctly

Project Member Reported by ClusterFuzz, Mar 29 2016

Issue description

Detailed report: https://cluster-fuzz.appspot.com/testcase?key=6672454977585152

Fuzzer: mbarbella_js_mutation_layout
Job Type: linux_debug_content_shell_drt
Platform Id: linux

Crash Type: ASSERT
Crash Address: 
Crash State:
  ASSERTION FAILED: !textNode->layoutObject() || textNode->layoutObject()->style()
  blink::InsertParagraphSeparatorCommand::doApply
  blink::CompositeEditCommand::applyCommandToComposite
  

Minimized Testcase (0.45 Kb):
Download: https://cluster-fuzz.appspot.com/download/AMIfv97ZlM_yNT1DAnx93CO_p1EPrDJ3K7w1ffgNKBtIEI4wAYX2ACotBruyf180pdGPeFuWTAG78OvCN2aIz_eoiL6MSrOyR-6b7XfFHIkIIYP7eYtPlVVe06zT02vYa__2qR5G9G0vgPBcMXt3LO8-V4exHVSrnA
<body onload="__f_631();" contenteditable="" font-size: 24px">
   <span id="test" style="white-space:pre">
    </span>
    z
  <script>
__v_266 = window.getSelection();
function __f_302() {
    __v_266.modify("move", "forward", "word");
}
function __f_275() {
    document.execCommand("JustifyRight");
}
function __f_343() {
    document.execCommand("InsertParagraph");
}
window.getSelection().collapse(test);
 __f_302(); 
 __f_275(); 
 __f_343(); 
</script>


Filer: durga.behera

See https://dev.chromium.org/Home/chromium-security/bugs/reproducing-clusterfuzz-bugs for more information.
 
Components: Blink>Editing
Labels: findit-for-crash Te-Logged
Owner: tkent@chromium.org
Status: Assigned (was: Available)
Suspected CLs	Regression information is not available. The result is the blame information.
-------------------------------
Author: dsinclair@chromium.org
Component: chromium
Changelist: https://chromium.googlesource.com/chromium/src//+/132c5d0b92d80a549926b2876b597990493b21d0
Time: Sat Mar 07 00:31:43 2015
The CL last changed line 397 of file InsertParagraphSeparatorCommand.cpp, which is stack frame 0.

Author: tkent
Component: chromium
Changelist: https://chromium.googlesource.com/chromium/src//+/141f0e9340ec887e341ba89a712c6539205a8292
Time: Tue Feb 09 12:09:23 2016
The CL last changed line 255 of file CompositeEditCommand.cpp, which is stack frame 1.

Author: tkent
Component: chromium
Changelist: https://chromium.googlesource.com/chromium/src//+/2e7914fe42c279c13e64f9bf07336d9c769ae3bf
Time: Wed Feb 17 10:04:46 2016
The CL last changed line 422 of file TypingCommand.cpp, which is stack frame 2.

Author: tkent
Component: chromium
Changelist: https://chromium.googlesource.com/chromium/src//+/2e7914fe42c279c13e64f9bf07336d9c769ae3bf
Time: Wed Feb 17 10:04:46 2016
The CL last changed line 281 of file TypingCommand.cpp, which is stack frame 3.

Author: tkent
Component: chromium
Changelist: https://chromium.googlesource.com/chromium/src//+/141f0e9340ec887e341ba89a712c6539205a8292
Time: Tue Feb 09 12:09:23 2016
The CL last changed line 208 of file CompositeEditCommand.cpp, which is stack frame 4.

Author: tkent
Component: chromium
Changelist: https://chromium.googlesource.com/chromium/src//+/832f566e42870da7a6e9df3c8a64c0e2f1f28b9e
Time: Wed Feb 24 03:48:19 2016
The CL last changed line 237 of file TypingCommand.cpp, which is stack frame 5.

Author: tkent
Component: chromium
Changelist: https://chromium.googlesource.com/chromium/src//+/832f566e42870da7a6e9df3c8a64c0e2f1f28b9e
Time: Wed Feb 24 03:48:19 2016
The CL last changed line 588 of file EditorCommand.cpp, which is stack frame 6.
-----------------------------------
Suspected Component: chromium
Suspected Cr- Label: Cr-Blink-Editing

From the above blame list suspecting the changes made to the file CompositeEditCommand.cpp, which is stack frame 1 and 4.
https://codereview.chromium.org/1679253002
tkent@ : Could you please take a look into this issue, if its related to your change.

Comment 2 by tkent@chromium.org, Mar 30 2016

Owner: ----
Status: Untriaged (was: Assigned)
Route to editing triage

Comment 3 by yosin@chromium.org, Mar 30 2016

Labels: -Pri-1 OS-Windows OS-All Pri-2
Status: Available (was: Untriaged)
Summary: InsertParagraphSeparatorCommand should handle whitespace:pre correctly (was: ASSERTION FAILED: !textNode->layoutObject() || textNode->layoutObject()->style())
Lower to Pri-2, since real world usage of InsertParagraphSeparatorCommand is low.

It seems this assertion is wrong. For leading whitespaces in whitespace:pre, we don't need to replace them to &nbsp;

DOM tree at assertion:
leadingWhitespace.showTreeForThis()
BODY	000004CAA8D03160 (editable) (focused)
	#text	000004CAA8D031C8 "\n   "
	SPAN	000004CAA8D03218 ID="test" STYLE="white-space:pre" (editable)
		#text	000004CAA8D034B8 "\n"
		DIV	000004CAA8D033E8 STYLE="text-align: right;" (editable)
			SPAN	000004CAA8D035C8 ID="test" (editable)
				#text	000004CAA8D03630 "    "
*			#text	000004CAA8D036E8 "\n    z"
	SCRIPT	000004CAA8D03320 (editable)
		#text	000004CAA8D03398 "...script..."

Comment 4 by yosin@chromium.org, Mar 30 2016

Minimum script to reproduce:
<body contenteditable>
<span style="white-space:pre"> </span><span id="test">z</span>
<script>
var selection = window.getSelection();
selection.collapse(test);
document.execCommand('InsertParagraph');
</script>

Comment 5 by yosin@chromium.org, Mar 30 2016

Owner: yosin@chromium.org
Status: Started (was: Available)
In review: http://crrev.com/1842753004
Project Member

Comment 6 by bugdroid1@chromium.org, Mar 31 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/07acbd0212cb0ae85e4828410648c04e94f3e3e2

commit 07acbd0212cb0ae85e4828410648c04e94f3e3e2
Author: yosin <yosin@chromium.org>
Date: Thu Mar 31 09:25:48 2016

Cleanup leadingWhitespacePosition()

This patch makes |leadingWhitespacePosition()| in early return style with
expanding |inSameContainingBlockFlowElement()| into it, which is used only
in here.

This patch is a preparation of http://crrev.com/1842753004.

BUG= 598665 
TEST=n/a; no behavior changes

Review URL: https://codereview.chromium.org/1847483002

Cr-Commit-Position: refs/heads/master@{#384229}

[modify] https://crrev.com/07acbd0212cb0ae85e4828410648c04e94f3e3e2/third_party/WebKit/Source/core/editing/EditingUtilities.cpp
[modify] https://crrev.com/07acbd0212cb0ae85e4828410648c04e94f3e3e2/third_party/WebKit/Source/core/editing/EditingUtilities.h

Project Member

Comment 7 by bugdroid1@chromium.org, Apr 1 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/2063467918ab92fe1969fbd9a1b37f29c4661cc6

commit 2063467918ab92fe1969fbd9a1b37f29c4661cc6
Author: yosin <yosin@chromium.org>
Date: Fri Apr 01 07:30:15 2016

Make leadingWhitespacePosition() to respect "white-space" CSS property

This patch makes |leadingWhitespacePosition()|, which returns collapsible
whitespace position, to return null |Position| when a previous position of
specified position is in a element which preserves whitespaces, e.g.
"white-space:pre", "white-space:pre-word", etc.

Before this patch, |leadingWhitespacePosition()| uses only
|isCollapsibleWhitespace(UChar)| to check collapsible whitespace without
considering "white-space" CSS property.

BUG= 598665 
TESET=run_layout_test editing/execCommand/insert_paragraph/insert_paragraph_pre_crash.html

Review URL: https://codereview.chromium.org/1854583003

Cr-Commit-Position: refs/heads/master@{#384509}

[add] https://crrev.com/2063467918ab92fe1969fbd9a1b37f29c4661cc6/third_party/WebKit/LayoutTests/editing/execCommand/insert_paragraph/insert_paragraph_pre_crash.html
[modify] https://crrev.com/2063467918ab92fe1969fbd9a1b37f29c4661cc6/third_party/WebKit/Source/core/editing/EditingUtilities.cpp
[modify] https://crrev.com/2063467918ab92fe1969fbd9a1b37f29c4661cc6/third_party/WebKit/Source/core/editing/EditingUtilities.h

Project Member

Comment 8 by ClusterFuzz, Apr 5 2016

Detailed report: https://cluster-fuzz.appspot.com/testcase?key=5911912134475776

Fuzzer: mbarbella_js_mutation_layout
Job Type: windows_syzyasan_content_shell
Platform Id: windows

Crash Type: UNKNOWN
Crash Address: 0x00000003
Crash State:
  blink::InsertParagraphSeparatorCommand::doApply
  blink::CompositeEditCommand::applyCommandToComposite
  blink::TypingCommand::insertParagraphSeparator
  
Regressed: https://cluster-fuzz.appspot.com/revisions?job=windows_syzyasan_content_shell&range=384825:384826

Unminimized Testcase: https://cluster-fuzz.appspot.com/download/AMIfv96a6vDpjQ2IjLphC46mGsgj1cH1EyzjuXtuYz5hkanWhmNH-0boUUWAX9gnyd0gBuDyHo_9gx80ilVFJDd2whBqXyOISBK0ciJdLzov6RGLA6OYq2Zsg_GJ_XoL8vmYU2eB2v75_WgtE90T257qAwIYxhKkMjO01qEsJGw6bKYTGG_14NM


Filer: pucchakayala

See https://dev.chromium.org/Home/chromium-security/bugs/reproducing-clusterfuzz-bugs for more information.

Comment 9 by yosin@chromium.org, Apr 5 2016

Status: Fixed (was: Started)
Project Member

Comment 10 by ClusterFuzz, Apr 6 2016

ClusterFuzz has detected this testcase as flaky and is unable to reproduce it in the original crash revision. Skipping fixed testing check and marking it as potentially fixed.

Detailed report: https://cluster-fuzz.appspot.com/testcase?key=5911912134475776

Fuzzer: mbarbella_js_mutation_layout
Job Type: windows_syzyasan_content_shell
Platform Id: windows

Crash Type: UNKNOWN
Crash Address: 0x00000003
Crash State:
  blink::InsertParagraphSeparatorCommand::doApply
  blink::CompositeEditCommand::applyCommandToComposite
  blink::TypingCommand::insertParagraphSeparator
  
Regressed: https://cluster-fuzz.appspot.com/revisions?job=windows_syzyasan_content_shell&range=384825:384826

Unminimized Testcase: https://cluster-fuzz.appspot.com/download/AMIfv96a6vDpjQ2IjLphC46mGsgj1cH1EyzjuXtuYz5hkanWhmNH-0boUUWAX9gnyd0gBuDyHo_9gx80ilVFJDd2whBqXyOISBK0ciJdLzov6RGLA6OYq2Zsg_GJ_XoL8vmYU2eB2v75_WgtE90T257qAwIYxhKkMjO01qEsJGw6bKYTGG_14NM


See https://dev.chromium.org/Home/chromium-security/bugs/reproducing-clusterfuzz-bugs for more information.

If you suspect that the result above is incorrect, try re-doing that job on the test case report page.
Project Member

Comment 11 by ClusterFuzz, Jun 9 2016

ClusterFuzz has detected this issue as fixed in range 384503:384535.

Detailed report: https://cluster-fuzz.appspot.com/testcase?key=6672454977585152

Fuzzer: mbarbella_js_mutation_layout
Job Type: linux_debug_content_shell_drt
Platform Id: linux

Crash Type: ASSERT
Crash Address: 
Crash State:
  !textNode->layoutObject() || textNode->layoutObject()->style()->collapseWhiteSpa
  blink::InsertParagraphSeparatorCommand::doApply
  blink::CompositeEditCommand::applyCommandToComposite
  
Fixed: https://cluster-fuzz.appspot.com/revisions?job=linux_debug_content_shell_drt&range=384503:384535

Minimized Testcase (0.47 Kb):
Download: https://cluster-fuzz.appspot.com/download/AMIfv96NwOe99JCC0zBaHdL6xv_KeM2-9JYIe-7Oc1cBJLxs90IQglQY3hZlKOGFadY69f584628ZyOqdoVChmw_X4ODSrde-wuQ5AmlJ3iPDv-Q9iKFMsNzf5lKrWx7IaGxkR9nm5UBdAx0cLpE6qP-32V5X9uQnA
<body onload="__f_631();" contenteditable="" font-size: 24px">
   <span id="test" style="white-space:pre">
    </span>
    z
  <script>
__v_266 = window.getSelection();
function __f_302() {
    __v_266.modify("move", "forward", "word");
}
function __f_275() {
    document.execCommand("JustifyRight");
}
function __f_343() {
    document.execCommand("InsertParagraph");
}
</script>
  <script>
window.getSelection().collapse(test);
 __f_302(); 
 __f_275(); 
 __f_343(); 
</script>


See https://dev.chromium.org/Home/chromium-security/bugs/reproducing-clusterfuzz-bugs for more information.

If you suspect that the result above is incorrect, try re-doing that job on the test case report page.
Project Member

Comment 12 by sheriffbot@chromium.org, Nov 22 2016

Labels: -Restrict-View-EditIssue
Removing EditIssue view restrictions from ClusterFuzz filed bugs. If you believe that this issue should still be restricted, please reapply the label.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

Sign in to add a comment