New issue
Advanced search Search tips

Issue 644147 link

Starred by 1 user
Status: Fixed
Owner:
yosin@chromium.org
Closed: Nov 2016
Cc:
editing-dev@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 2
Type: Bug



Sign in to add a comment

ReplaceSelectionCommand::doApply has a wrong assertion

Project Member Reported by yosin@chromium.org, Sep 6 2016 
Following test causes following DCHECK:
Check failed: enclosingBlockOfInsertionPos != currentRoot (DIV (editable) vs. DIV (editable))

at replaceselectioncommand.cpp(1151)

assert_selection(
        [
            '<div contenteditable>',
                'one',
                '<div>two^</div>',
                '^t|hree',
           '</div>',
        ].join(''),
        selection => {
            selection.document.execCommand('copy');
            selection.document.execCommand('paste');
            selection.document.execCommand('paste');
        },
        'TBD');

Comment 1 by yosin@chromium.org, Sep 9 2016

Summary: ReplaceSelectionCommand::doApply has a wrong assertion (was: ReplaceSelectionCommand crashes with interchange-newline)
The wrong assertion is
  DCHECK_NE(enclosingBlockOfInsertionPos, currentRoot);
in ReplaceSelection::doApply()

This assertion hits the test case in #c1 which is converted from paste-text-009.html in LayoutTests/editing/pasteboard.

In assertion, #c1
enclosingBlockOfInsertionPos is <div>one
currentRoot is <div>one

In paste-text-009.html
enclosingBlockOfInsertionPos <div>Ommited
currentRoot=BODY
Project Member

Comment 2 by bugdroid1@chromium.org, Sep 12 2016 

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

commit 8f313902e23e2589ddd9d3d2bfed8ee61e979f17
Author: yosin <yosin@chromium.org>
Date: Mon Sep 12 07:41:20 2016

Convert an assertion to if-condition in ReplaceSelectionCommand::doApply()

This patch converts a following assertion
  DCHECK_NE(enclosingBlockOfInsertionPos, currentRoot)
to if-condition in |ReplaceSelectionCommand::doApply()|, since we found a
pattern to hit this assertion in "paste-text.html".

New test case is the result of converting "paste-text-009.html" to use w3c
test harness. The difference between "paste-test.html" and "paste-text-009.html"
is root editable:
  "past-text.html":
    |currentRoot| is "<div>one"
    |enclosingBlockOfInsertionPos| is "<div>one"

  "paste-text-009.html":
    |currentRoot| is BODY
    |enclosingBlockOfInsertionPos| is "<div>Omitted".

This patch also changes a variable name reference |insertionPos| to
|enclosingBlockOfInsertionPos| to match with current code.

Note: This assertion is introduced by https://bugs.webkit.org/show_bug.cgi?id=8592

BUG= 644147 
TEST=LayoutTests/editing/pasteboard/paste_text.html

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

[delete] https://crrev.com/8879e683401bee738ac857c99e6e109911aa62dd/third_party/WebKit/LayoutTests/editing/pasteboard/paste-text-009.html
[modify] https://crrev.com/8f313902e23e2589ddd9d3d2bfed8ee61e979f17/third_party/WebKit/LayoutTests/editing/pasteboard/paste_text.html
[delete] https://crrev.com/8879e683401bee738ac857c99e6e109911aa62dd/third_party/WebKit/LayoutTests/platform/linux/editing/pasteboard/paste-text-009-expected.png
[delete] https://crrev.com/8879e683401bee738ac857c99e6e109911aa62dd/third_party/WebKit/LayoutTests/platform/linux/editing/pasteboard/paste-text-009-expected.txt
[delete] https://crrev.com/8879e683401bee738ac857c99e6e109911aa62dd/third_party/WebKit/LayoutTests/platform/mac/editing/pasteboard/paste-text-009-expected.png
[delete] https://crrev.com/8879e683401bee738ac857c99e6e109911aa62dd/third_party/WebKit/LayoutTests/platform/mac/editing/pasteboard/paste-text-009-expected.txt
[delete] https://crrev.com/8879e683401bee738ac857c99e6e109911aa62dd/third_party/WebKit/LayoutTests/platform/win/editing/pasteboard/paste-text-009-expected.png
[delete] https://crrev.com/8879e683401bee738ac857c99e6e109911aa62dd/third_party/WebKit/LayoutTests/platform/win/editing/pasteboard/paste-text-009-expected.txt
[modify] https://crrev.com/8f313902e23e2589ddd9d3d2bfed8ee61e979f17/third_party/WebKit/Source/core/editing/commands/ReplaceSelectionCommand.cpp

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

Status: Fixed (was: Started)
►

Sign in to add a comment