New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 619131 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Jul 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Windows
Pri: 1
Type: Bug

Blocked on:
issue 625533



Sign in to add a comment

Paste command should handle empty SPAN elements correctly

Project Member Reported by ClusterFuzz, Jun 10 2016

Issue description

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

Fuzzer: inferno_layout_test_unmodified
Job Type: windows_syzyasan_content_shell
Platform Id: windows

Crash Type: UNKNOWN
Crash Address: 0x0000000b
Crash State:
  blink::SimplifyMarkupCommand::doApply
  blink::CompositeEditCommand::applyCommandToComposite
  blink::ReplaceSelectionCommand::doApply
  
Regressed: https://cluster-fuzz.appspot.com/revisions?job=windows_syzyasan_content_shell&range=398628:398752

Minimized Testcase (1.24 Kb): https://cluster-fuzz.appspot.com/download/AMIfv94U8AeXjCIFq8sK3zUQ1JYbQ4QAROa-T_M73JrpNuDZh2mHtE_yfafB0yJqEUz005Wwe_WHT50ifkzySjmXOPQKdArAyLdgCF07NYCzzbMrPRS-VFiMlH71j_26Cgf6yiQDviO3-c5Vb8eEe9Xi71wi6JEFQg

Filer: mummareddy

See https://dev.chromium.org/Home/chromium-security/bugs/reproducing-clusterfuzz-bugs for more information.
 
Labels: findit-wrong Te-Logged M-53
Owner: tkent@chromium.org
Status: Assigned (was: Available)
From code search on file SimplifyMarkupCommand.cpp
suspected CL's are 

https://chromium.googlesource.com/chromium/src/+/2185123a479eddd9e20a97b89b23619e79a7af09
https://chromium.googlesource.com/chromium/src/+/7f6bd2b6a8e6e4858afd1f1b23d768030a01af69

tkent@, could you please have a look and help us to find owner if it is not related your changes.
Thank you.

Comment 2 by tkent@chromium.org, Jun 12 2016

Components: Blink>Editing
Owner: ----
Status: Untriaged (was: Assigned)
Route to Blink>Editing triage

Comment 3 by yosin@chromium.org, Jun 27 2016

Components: -Blink>Editing Blink>Editing>Paste
Status: Available (was: Untriaged)
removeRedundantStylesAndKeepStyleSpanInline() in ReplaceSelectionCommand class makes insertNodes.m_firstNodeINserted to nullptr, then SimplifyMarkupCommand() uses insertNodes.m_firstNodeINserted.

Given that, we should check whether insertNodes.m_firstNodeINserted is nullptr or not before calling SimplifyMarkupCommand().

Comment 4 by yosin@chromium.org, Jul 6 2016

Blockedon: 625533
Summary: updatePositionForNodeRemoval() should handle AfterAnchor position correctly (was: Crash in blink::SimplifyMarkupCommand::doApply)
The root cause is (OBJECT, AfterAnchor) handling in updatePositionForNodeRemoval()
called by FrameSelection::nodeWillBeRemoved().

In below script, selection should (BODY, 0) after calling Range::deleteContents()


# Minimal script:
<!doctype html>
<body><object></object></body>
<script>
setTimeout(function(){
    document.execCommand('SelectAll');
    document.addEventListener('copy', event => {
        event.clipboardData.setData('text/html', '<span></span><span></span>');
        event.clipboardData.setData('text/plain', '');
        event.preventDefault();
    });
    // Put empty SPAN element with STYLE attribute into clipboard.
    document.execCommand('Copy');
    var range = new Range();
    range.setStart(document.body, 0);
    range.setEnd(document.body, 1);
    // VS.end is OBJECT, AfterAnchor
    // relocation it to BODY, 1
    range.deleteContents();
    document.designMode = 'on';
    document.execCommand('paste');
});
</script>

Comment 5 by yosin@chromium.org, Jul 6 2016

Owner: yosin@chromium.org
Status: Started (was: Available)
Correction: Root cause is "paste" (ReplaceSelectionCommand), position relocation
isn't related. #c3 is correct.

Comment 6 by yosin@chromium.org, Jul 6 2016

Summary: Paste command should handle empty SPAN elements correctly (was: updatePositionForNodeRemoval() should handle AfterAnchor position correctly)
Project Member

Comment 7 by bugdroid1@chromium.org, Jul 6 2016

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

commit b0ff83a86d043140c423a47c164d43fab5a552bb
Author: yosin <yosin@chromium.org>
Date: Wed Jul 06 12:08:07 2016

Make previousPositionOf() to handle yet another no previous position case

This patch makes |previousPositionOf()| to handle no previous position case,
which is occurred after visible position canonicalization.

It is time to change |DCEHCK()| to return nothing since we have a test case
for it.

BUG= 619131 
TEST=run_webkit_unit_tests --gtest_filter=VisibleUnitTests.previousPositionOfNoPreviousPosition

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

[modify] https://crrev.com/b0ff83a86d043140c423a47c164d43fab5a552bb/third_party/WebKit/Source/core/editing/VisibleUnits.cpp
[modify] https://crrev.com/b0ff83a86d043140c423a47c164d43fab5a552bb/third_party/WebKit/Source/core/editing/VisibleUnitsTest.cpp

Project Member

Comment 8 by sheriffbot@chromium.org, Jul 6 2016

Labels: -M-53 M-54 MovedFrom-53
Moving this nonessential bug to the next milestone.

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

Comment 9 by yosin@chromium.org, Jul 7 2016

Status: Fixed (was: Started)
Project Member

Comment 10 by bugdroid1@chromium.org, Jul 7 2016

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

commit cd80804c61b6d9578a633d3546043d03aecdf27b
Author: yosin <yosin@chromium.org>
Date: Thu Jul 07 04:41:48 2016

Make ReplaceSelectionCommand to handle empty SPAN replacement

This patch makes |ReplaceSelectionCommand| to handle empty SPAN replacement.

Before this patch, it crashes at |SimplifyMarkupCommand::doApply()| with
null pointer reference passed by |ReplaceSelectionCommand| with
null |insertNodes.firstNodeInserted()|.

BUG= 619131 
TEST=run_webkit_unit_tests --gtest_filter=ReplaceSelectionCommandTest.pastingEmptySpan

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

[modify] https://crrev.com/cd80804c61b6d9578a633d3546043d03aecdf27b/third_party/WebKit/Source/core/editing/commands/ReplaceSelectionCommand.cpp
[modify] https://crrev.com/cd80804c61b6d9578a633d3546043d03aecdf27b/third_party/WebKit/Source/core/editing/commands/ReplaceSelectionCommandTest.cpp

Project Member

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