New issue
Advanced search Search tips

Issue 753657 link

Starred by 1 user

Issue metadata

Status: Available
Owner: ----
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Task

Blocked on:
issue 754012



Sign in to add a comment

Convert text-iterator layout test to gtest

Project Member Reported by yosin@chromium.org, Aug 9 2017

Issue description

There are no good reasons using layout test for verifying TextIterator.
We should convert below to gtest:

LayoutTests/editing/text-iterator/

backward-textiterator-first-letter-crash.html
basic-iteration-shadowdom.html
basic-iteration.html
find-after-mutation.html
findString-restarts-at-last-position.html
findString-selection-disabled.html
findString-shadow-roots.html
findString-start-search-after-selection.html
findString.html
first-letter-rtl-crash.html
first-letter-word-boundary.html
range-to-from-location-and-length.html
read-past-cloned-first-letter.html
rtl-first-letter-text-iterator-crash.html
rtl-selection-crash.html
selection-to-string-with-auto-fill.html
thai-cursor-movement.html

 
There is a blocking issue to be fixed that, currently, layout tests and unit tests generate different line box trees. Since a lot of TextIterator behaviors depend on the actual line layout, the issue has to be fixed to unblock the conversion.

Example HTML (repro case grabbed from  crbug.com/752941 ):

<style>
font {
  -webkit-appearance: progress-bar;
}

:first-letter {
  color:black
}

* {
  -webkit-rtl-ordering: visual;
  -webkit-margin-end: -1px
}
</style>
<font dir="rtl">iC <q></q></font>

Layout test generates:

LayoutBlockFlow 0x358371a24250         	FONT
  RootInlineBox 0x358371a300d8         	LayoutBlockFlow 0x358371a24250 {pos=17,0 size=15,19} baseline=15/10
*   InlineTextBox 0x358371a64010       	LayoutTextFragment 0x358371a443d0                                         (0,1) "C"
    InlineTextBox 0x358371a64090       	LayoutTextFragment 0x358371a444c0                                         (0,1) "i"
  RootInlineBox 0x358371a301a0         	LayoutBlockFlow 0x358371a24250 {pos=19,20 size=13,19} baseline=15/10
    InlineFlowBox 0x358371a681d8       	LayoutInline 0x358371a40010 {pos=18,20 size=14,19} baseline=15/10
      InlineFlowBox 0x358371a68270     	LayoutInline 0x358371a40180 {pos=18,20 size=7,19} baseline=15/10
        InlineFlowBox 0x358371a68140   	LayoutQuote (anonymous) 0x358371a441f0 {pos=18,20 size=7,19} baseline=15/10
          InlineTextBox 0x358371a64190 	LayoutTextFragment 0x358371a442e0                                         (0,1) """
      InlineFlowBox 0x358371a68010     	LayoutInline 0x358371a400c8 {pos=25,20 size=7,19} baseline=15/10
        InlineFlowBox 0x358371a680a8   	LayoutQuote (anonymous) 0x358371a44010 {pos=25,20 size=7,19} baseline=15/10
          InlineTextBox 0x358371a64110 	LayoutTextFragment 0x358371a44100                                         (0,1) """


Unit test generates:

LayoutBlockFlow 0x1d3249e24250         	FONT
  RootInlineBox 0x1d3249e30010         	LayoutBlockFlow 0x1d3249e24250 {pos=1,0 size=0,1} baseline=1/1
    InlineFlowBox 0x1d3249e68140       	LayoutInline 0x1d3249e40010 {pos=0,0 size=0,1} baseline=1/1
      InlineFlowBox 0x1d3249e680a8     	LayoutInline 0x1d3249e40180 {pos=0,0 size=0,1} baseline=1/1
        InlineFlowBox 0x1d3249e68010   	LayoutQuote (anonymous) 0x1d3249e441f0 {pos=0,0 size=0,1} baseline=1/1
          InlineTextBox 0x1d3249e64010 	LayoutTextFragment 0x1d3249e442e0                                         (0,1) """
      InlineFlowBox 0x1d3249e68270     	LayoutInline 0x1d3249e400c8 {pos=0,0 size=0,1} baseline=1/1
        InlineFlowBox 0x1d3249e681d8   	LayoutQuote (anonymous) 0x1d3249e44010 {pos=0,0 size=0,1} baseline=1/1
          InlineTextBox 0x1d3249e64090 	LayoutTextFragment 0x1d3249e44100                                         (0,1) """
*   InlineTextBox 0x1d3249e64110       	LayoutTextFragment 0x1d3249e443d0                                         (0,2) "C "
    InlineTextBox 0x1d3249e64190       	LayoutTextFragment 0x1d3249e444c0                                         (0,1) "i"
Blockedon: 754012
The text dumps in layout tests also form a test suite for TextIterator, which, however, shouldn't be relied on.

During the development of TextIterator NG, some existing bugs in TextIterator are found, and a number of layout tests are rebaselined. We should also convert the rebaselined layout tests (somehow) to unit tests, so that the correct behavior of TextIterator NG is coverted by some tests dedicated to it.

The list of rebaselined tests can be found in https://chromium-review.googlesource.com/c/580371
Owner: akariasai@google.com
Project Member

Comment 5 by bugdroid1@chromium.org, Aug 29 2017

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

commit 2266a8ad92bf914bb090d036a2619c41a953c3a9
Author: Akari Asai <akariasai@google.com>
Date: Tue Aug 29 07:34:24 2017

Converts basic-iteration.js to gtest.

Converted WebKit/LayoutTests/editing/text-iterator/script-tests/basic-iteration.js
to gtest and added the corresponding tests to WebKit/Source/core/editing/iterators/
TextIteratorTest.cpp, since there are no good reasons using layout test for verifying
TextIterator.

Bug: 753657
Change-Id: I392b13da25829ab312d82eb7c0dcb6404a5de300
Reviewed-on: https://chromium-review.googlesource.com/628398
Reviewed-by: Xiaocheng Hu <xiaochengh@chromium.org>
Commit-Queue: Akari Asai <akariasai@google.com>
Cr-Commit-Position: refs/heads/master@{#498041}
[modify] https://crrev.com/2266a8ad92bf914bb090d036a2619c41a953c3a9/third_party/WebKit/LayoutTests/FlagExpectations/enable-blink-features=LayoutNG
[delete] https://crrev.com/a6df92167f0eabd6496a84877ba80f66a928ecbd/third_party/WebKit/LayoutTests/editing/text-iterator/basic-iteration-expected.txt
[delete] https://crrev.com/a6df92167f0eabd6496a84877ba80f66a928ecbd/third_party/WebKit/LayoutTests/editing/text-iterator/basic-iteration.html
[delete] https://crrev.com/a6df92167f0eabd6496a84877ba80f66a928ecbd/third_party/WebKit/LayoutTests/editing/text-iterator/script-tests/basic-iteration.js
[modify] https://crrev.com/2266a8ad92bf914bb090d036a2619c41a953c3a9/third_party/WebKit/Source/core/editing/iterators/TextIteratorTest.cpp

Project Member

Comment 6 by sheriffbot@chromium.org, Oct 2 2017

Labels: Hotlist-Recharge-BouncingOwner
Owner: ----
Status: Untriaged (was: Available)
The assigned owner "akariasai@google.com" is not able to receive e-mails, please re-triage.

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

Comment 7 by yosin@chromium.org, Oct 3 2017

Status: Available (was: Untriaged)
Project Member

Comment 8 by sheriffbot@chromium.org, Oct 3

Labels: Hotlist-Recharge-Cold
Status: Untriaged (was: Available)
This issue has been Available for over a year. If it's no longer important or seems unlikely to be fixed, please consider closing it out. If it is important, please re-triage the issue.

Sorry for the inconvenience if the bug really should have been left as Available.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Status: Available (was: Untriaged)

Sign in to add a comment