New issue
Advanced search Search tips

Issue 630921 link

Starred by 1 user

Issue metadata

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



Sign in to add a comment

Text iterator ignores ending positions that are in invisible subtrees

Project Member Reported by xiaoche...@chromium.org, Jul 25 2016

Issue description

Version: ToT (54.0.2807.0)
OS: All

The following unit test, if introduced, fails:

TEST_F(TextIteratorTest, EndingConditionWithDisplayNone)
{
    setBodyContent("<div style='display: none'><span>hello</span>world</div>Lorem ipsum dolor sit amet.");
    Position start(&document(), 0);
    Position end(document().querySelector("span", ASSERT_NO_EXCEPTION), 0);
    TextIterator iter(start, end);
    EXPECT_TRUE(iter.atEnd());
}

The root cause is that both the ending position and its succeeding position are in an invisible subtree, which gets skipped by the text iterator.

The bug is revealed by cluster fuzz in  issue 597628  by a flaky case, which no longer reproduces now. The root cause is still there, though, as shown by the unit test.
 
 Issue 597628  has been merged into this issue.
Project Member

Comment 2 by bugdroid1@chromium.org, Jul 27 2016

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

commit 32dad58c08112ed4a04505675aca27d37d2ce755
Author: xiaochengh <xiaochengh@chromium.org>
Date: Wed Jul 27 07:18:49 2016

Ensure that TextIterator::m_pastEndNode is not skipped in advance()

In the current implemetation of |TextIterator::advance()|, when facing a
node that is neither a shadow root nor a laid-out node, the node and its
entire subtree are directly skipped, which is incorrect if the ending
position is in the subtree.

This CL fixes the issue by setting |m_pastEndNode| as the first non-skippable
node after the ending position. It also ensures that |m_pastEndNode| is
only used for checking loop ending condition, and replaces the other usage
with an equivalent implementation.

BUG= 630921 
TEST=webkit_unit_tests --gtest_filter=TextIteratorTest.EndingConditionWithDisplayNone*

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

[modify] https://crrev.com/32dad58c08112ed4a04505675aca27d37d2ce755/third_party/WebKit/Source/core/editing/iterators/TextIterator.cpp
[modify] https://crrev.com/32dad58c08112ed4a04505675aca27d37d2ce755/third_party/WebKit/Source/core/editing/iterators/TextIterator.h
[modify] https://crrev.com/32dad58c08112ed4a04505675aca27d37d2ce755/third_party/WebKit/Source/core/editing/iterators/TextIteratorTest.cpp

Status: Fixed (was: Started)

Sign in to add a comment