third_party/WebKit/LayoutTests/paint/invalidation/selection/selection-background-repaint.html - Issue 2709693003: Repaint selection when element with ::selection style is recalculated.

Side by Side Diff: third_party/WebKit/LayoutTests/paint/invalidation/selection/selection-background-repaint.html

Issue 2709693003: Repaint selection when element with ::selection style is recalculated. (Closed)

Patch Set: Created 3 years, 10 months ago

Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.

Jump to:

« no previous file with comments | « no previous file | third_party/WebKit/LayoutTests/paint/invalidation/selection/selection-background-repaint-expected.html » ('j') | third_party/WebKit/Source/core/layout/LayoutObject.cpp » ('J')
Toggle Intra-line Diffs ('i') | Expand Comments ('e') | Collapse Comments ('c') | Hide Comments ('s')

OLD	NEW
(Empty)
	1 <!DOCTYPE html>

	2 <style>

	3 ::selection {

	4 background-color: transparent;

	5 color: red

	6 }

	7 .green::selection {

	8 color: green

	9 }

	10 </style>

	11 <p>The words GREEN below should be green.</p>

	12 <span id="t1">GREEN<span> RED </span>GREEN</span>
	mstensho (USE GERRIT) 2017/02/21 21:03:54 The fact that "RED" is indeed painted in red here The fact that "RED" is indeed painted in red here is an implementation-specific thing, right (according to the commit comment)? So that if someone decides to fix that, this test would break? Is it really necessary to depend on this kind of shortcoming in Blink to test your fix? I checked, and without your code changes, the test also seems to fail with just an empty <span> between the two GREEN words. rune 2017/02/21 21:42:00 Yes, it was to prove to the reader of the test and Show quoted text On 2017/02/21 21:03:54, mstensho wrote: > The fact that "RED" is indeed painted in red here is an implementation-specific > thing, right (according to the commit comment)? So that if someone decides to > fix that, this test would break? Yes, it was to prove to the reader of the test and code that the shallow layout tree walk for leaves was enough, but it would also catch the inadequacy of this code if the spec ends up differently. No-one's been able to specify this yet. ::selection was in a Selectors Level 3 CR, under-specified, but was removed later. Also see https://drafts.csswg.org/css-pseudo-4/#highlight-cascade Show quoted text > Is it really necessary to depend on this kind of shortcoming in Blink to test > your fix? I checked, and without your code changes, the test also seems to fail > with just an empty <span> between the two GREEN words. No. I can simplify it. rune 2017/02/21 22:34:57 Done. Show quoted text On 2017/02/21 21:42:00, rune wrote: > On 2017/02/21 21:03:54, mstensho wrote: > > The fact that "RED" is indeed painted in red here is an > implementation-specific > > thing, right (according to the commit comment)? So that if someone decides to > > fix that, this test would break? > > Yes, it was to prove to the reader of the test and code that the shallow layout > tree walk for leaves was enough, but it would also catch the inadequacy of this > code if the spec ends up differently. No-one's been able to specify this yet. > ::selection was in a Selectors Level 3 CR, under-specified, but was removed > later. Also see https://drafts.csswg.org/css-pseudo-4/#highlight-cascade > > > Is it really necessary to depend on this kind of shortcoming in Blink to test > > your fix? I checked, and without your code changes, the test also seems to > fail > > with just an empty <span> between the two GREEN words. > > No. I can simplify it. Done.
	13 <script>

	14 if (window.testRunner)

	15 testRunner.waitUntilDone();

	16

	17 function setSelection(startTextNode, endTextNode) {

	18 window.getSelection().removeAllRanges();

	19 var range = document.createRange();

	20 range.setStart(startTextNode, 0);

	21 range.setEnd(endTextNode, endTextNode.data.length);

	22 window.getSelection().addRange(range);

	23 }

	24

	25 setSelection(t1.firstChild, t1.lastChild);

	26 requestAnimationFrame(() =>

	27 requestAnimationFrame(() => {

	28 t1.className = "green";

	29 if (window.testRunner)

	30 testRunner.notifyDone();

	31 }));

	32 </script>

OLD	NEW