New issue
Advanced search Search tips

Issue 663613 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Nov 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Android , Windows , Chrome , Mac
Pri: 1
Type: Bug



Sign in to add a comment

reconcileTextDecorationProperties() should handle styles having both text-decoration and webkit-text-decoration-in-effect

Project Member Reported by ClusterFuzz, Nov 9 2016

Issue description

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

Fuzzer: inferno_layout_test_unmodified
Job Type: linux_debug_content_shell_drt
Platform Id: linux

Crash Type: CHECK failure
Crash Address: 
Crash State:
  !textDecorationsInEffect || !textDecoration in EditingStyle.cpp
  blink::reconcileTextDecorationProperties
  blink::StyleChange::StyleChange
  
Regressed: https://cluster-fuzz.appspot.com/revisions?job=linux_debug_content_shell_drt&range=268656:269696

Minimized Testcase (0.24 Kb):
Download: https://cluster-fuzz.appspot.com/download/AMIfv94FtO30BFWkl0dblpQHStwhK0tcbzcd7wQyuHIgkXvN6W498hcZNos0nDMzJ8sjqJ5Ji5EC4BPUAimBz9dfVRg3pHVANawGwOjUYSH2Mgv9VEG6_mDb2uDI0fMg_NmypUOLaSb-eqJOJ2KYeZBdPMDkMVDnjw?testcase_id=6702977839792128
        Female
  <dl>
    DT element
  </dl>
  <br/>
  <hr/>
  <script>
    document.designMode = 'on';
    document.execCommand('SelectAll');
    document.execCommand('Strikethrough');
    document.execCommand('InsertOrderedList');
</script>


Issue filed automatically.

See https://dev.chromium.org/Home/chromium-security/bugs/reproducing-clusterfuzz-bugs for more information.
 
Components: Blink>Editing Blink>CSS
Labels: M-54 Test-Predator-Wrong M-55
Owner: xiaoche...@chromium.org
Status: Assigned (was: Untriaged)
Suspected CL
https://codereview.chromium.org/2422133002

xiaochengh@, could you please take a look and help us to find correct owner if it is not related your changes.

Cc: yosin@chromium.org
Components: -Blink>Editing Blink>Editing>Command
A further minimized test case with assert_selection:

test(() => assert_selection(
    [
      '<div contenteditable>',
        '<strike>^Female</strike>',
        '<dl><strike>DT element</strike></dl>',
        '<strike><br></strike>',
        '<hr>|',
      '</div>'
    ].join(''),
    '',
    ''));

No idea why "We shouldn't have both text-decoration and -webkit-text-decorations-in-effect because that wouldn't make sense" (quoted from EditingStyle.cpp just above the failed DCHECK)...
Project Member

Comment 4 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

Comment 5 by yosin@chromium.org, Nov 28 2016

It seems "-webkit-text-decorations-in-effect" CSS property is deprecated, http://crbug.com/269140

Comment 6 by yosin@chromium.org, Nov 28 2016

Labels: OS-Android OS-Chrome OS-Mac OS-Windows
Below test case causes DCHECK(!textDecorationsInEffect || !textDecoration);

test(() => assert_selection(
  [
    '<div contenteditable>',
      '<strike>^Female</strike>',
      '<dl><strike>DT element</strike></dl>',
      '<strike><br></strike>',
      '<hr>|',
    '</div>'
  ].join(''),
  'insertOrderedList',
  [
    '<div contenteditable>',
      '<ol><li><strike>^Female</strike></li></ol>',
      '<dl>',
        '<ol>',
          '<ul><strike>DT element</strike></ul>',
          '<li><strike><br></strike></li>',
          '<li><hr>|</li>',
        '</ol>',
      '</dl>',
    '</div>'
  ].join('')),
  'insertOrderedList with STRIKE, DL, and HR');

Comment 7 by yosin@chromium.org, Nov 28 2016

"-webkit-text-decorations-in-effect" is introduced by moveParagraphs() via StyledMarkupSerializer for serializing STRIKE element.

Comment 8 by yosin@chromium.org, Nov 28 2016

Cc: -yosin@chromium.org
Components: -Blink>CSS
Owner: yosin@chromium.org
Status: Started (was: Assigned)
Summary: reconcileTextDecorationProperties() should handle styles having both text-decoration and webkit-text-decoration-in-effect (was: !textDecorationsInEffect || !textDecoration in EditingStyle.cpp)
In review: http://crrev.com/2532873002
Project Member

Comment 9 by bugdroid1@chromium.org, Nov 29 2016

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

commit a9814d2d1bbc0c1d77dad4f2619e21e28757b47f
Author: yosin <yosin@chromium.org>
Date: Tue Nov 29 03:05:02 2016

Get rid of wrong assumption in reconcileTextDecorationProperties()

This patch gets rid of |DCHECK()|, which is a wrong assumption, in
|reconcileTextDecorationProperties()| since we have a test case against the
assumption.

BUG= 663613 
TEST=LayoutTests/editing/execCommand/insert-list-and-strikethrough.html

Review-Url: https://codereview.chromium.org/2532873002
Cr-Commit-Position: refs/heads/master@{#434857}

[add] https://crrev.com/a9814d2d1bbc0c1d77dad4f2619e21e28757b47f/third_party/WebKit/LayoutTests/editing/execCommand/insert-list-and-strikethrough.html
[modify] https://crrev.com/a9814d2d1bbc0c1d77dad4f2619e21e28757b47f/third_party/WebKit/Source/core/editing/EditingStyle.cpp

Comment 10 by yosin@chromium.org, Nov 29 2016

Status: Fixed (was: Started)
Project Member

Comment 11 by ClusterFuzz, Nov 29 2016

ClusterFuzz has detected this issue as fixed in range 434840:434865.

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

Fuzzer: inferno_layout_test_unmodified
Job Type: linux_debug_content_shell_drt
Platform Id: linux

Crash Type: CHECK failure
Crash Address: 
Crash State:
  !textDecorationsInEffect || !textDecoration in EditingStyle.cpp
  blink::reconcileTextDecorationProperties
  blink::StyleChange::StyleChange
  
Regressed: https://cluster-fuzz.appspot.com/revisions?job=linux_debug_content_shell_drt&range=268656:269696
Fixed: https://cluster-fuzz.appspot.com/revisions?job=linux_debug_content_shell_drt&range=434840:434865

Minimized Testcase (0.24 Kb):
Download: https://cluster-fuzz.appspot.com/download/AMIfv94FtO30BFWkl0dblpQHStwhK0tcbzcd7wQyuHIgkXvN6W498hcZNos0nDMzJ8sjqJ5Ji5EC4BPUAimBz9dfVRg3pHVANawGwOjUYSH2Mgv9VEG6_mDb2uDI0fMg_NmypUOLaSb-eqJOJ2KYeZBdPMDkMVDnjw?testcase_id=6702977839792128
        Female
  <dl>
    DT element
  </dl>
  <br/>
  <hr/>
  <script>
    document.designMode = 'on';
    document.execCommand('SelectAll');
    document.execCommand('Strikethrough');
    document.execCommand('InsertOrderedList');
</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.

Sign in to add a comment