New issue
Advanced search Search tips

Issue 697989 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Mar 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

Unit tests in EphemeralTest.cpp are badly written

Project Member Reported by xiaoche...@chromium.org, Mar 2 2017

Issue description

These tests use the following HTML:

<p id='host'>
  <b id='one'></b>
  <input type='text' value='some'>
</p>

This is bad because:

1. These tests introduce extra dependency on the UA shadow dom under <input>, which is irrelevant to EphemeralRange
2. <p> is not a shadow host, but is carrying a misleading id 'host'
3. Empty <b> node

We should use an alternative HTML with a shadow dom explicitly attached to it, and also remove the misleading parts.
 
Can i take this up ? I have some background idea behind this as i worked on 691201.

Comment 2 by yosin@chromium.org, Mar 8 2017

Owner: yosin@chromium.org
Status: Started (was: Available)
tanvir.rizvi@, yes please!

Set myself as virtual owner for tracking.

Comment 3 by yosin@chromium.org, Mar 10 2017

tanvir.rizvi works this: crrev.com/2738203002

Project Member

Comment 4 by bugdroid1@chromium.org, Mar 10 2017

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

commit 7cb26a09a58fbbf6dc28d3a475757445756fd46f
Author: tanvir.rizvi <tanvir.rizvi@samsung.com>
Date: Fri Mar 10 22:12:27 2017

Update EphemeralRange Unit Tests

EphemeralRange unit tests updated with better HTML and shadow-DOM
attached  explicitly. Previously few of the test cases introduced
extra dependency on the UA shadow DOM under <input>,
which was irrelevant to EphemeralRange.

BUG= 697989 

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

[modify] https://crrev.com/7cb26a09a58fbbf6dc28d3a475757445756fd46f/third_party/WebKit/Source/core/editing/EphemeralRangeTest.cpp

Status: Fixed (was: Started)
Fixed by #4

Sign in to add a comment