third_party/WebKit/LayoutTests/paint/invalidation/ruby-text-flipped-blocks.html - Issue 2150143002: Don't over-flip when mapping visual rects for Ruby text.

Side by Side Diff: third_party/WebKit/LayoutTests/paint/invalidation/ruby-text-flipped-blocks.html

Issue 2150143002: Don't over-flip when mapping visual rects for Ruby text. (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master

Patch Set: Add tests. Created 4 years, 5 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:

View unified diff | Download patch

« third_party/WebKit/LayoutTests/paint/invalidation/ruby-text.html ('K') | « third_party/WebKit/LayoutTests/paint/invalidation/ruby-text-expected.txt ('k') | third_party/WebKit/LayoutTests/paint/invalidation/ruby-text-flipped-blocks-expected.txt » ('j') | no next file with comments »
Toggle Intra-line Diffs ('i') | Expand Comments ('e') | Collapse Comments ('c') | Hide Comments ('s')

OLD	NEW
(Empty)
	1 <!doctype html>

	2 <script src="../../fast/repaint/resources/text-based-repaint.js"></script>

	3 <div style="writing-mode: vertical-rl;">

	4 <ruby id="outside">b<rt id="inside">c</rt></ruby>
	chrishtr 2016/07/15 01:24:29 Could you quickly summarize the flips that occur h Could you quickly summarize the flips that occur here? Is it <rt> flipping into <ruby> and since <ruby> is inline, it shouldn't flip into <div>? Is this a more general problem with inlines? What happens if you nest spans, or use display: inline-block? wkorman 2016/07/15 01:59:23 I think all of the general cases are fine. The pro Show quoted text On 2016/07/15 at 01:24:29, chrishtr wrote: > Could you quickly summarize the flips that occur here? Is it <rt> flipping into <ruby> and since <ruby> is inline, it shouldn't > flip into <div>? > > Is this a more general problem with inlines? What happens if you nest spans, or use display: inline-block? I think all of the general cases are fine. The problem is due to what is essentially our internal implementation detail of Ruby text, where the test here expands into a layout tree like: LayoutBlockFlow {DIV} at (0,0) size 32x8 LayoutRuby (inline) {RUBY} at (0,0) size 19x8 LayoutRubyRun (anonymous) at (12,0) size 20x8 LayoutRubyText {RT} at (-12,0) size 12x8 LayoutText {#text} at (0,2) size 12x4 text run at (0,2) width 4: "c" LayoutRubyBase (anonymous) at (0,0) size 20x8 LayoutText {#text} at (0,0) size 19x8 text run at (0,0) width 8: "b" The bug led to incorrect visual rect for the InlineTextBoxes under the LayoutTexts. Control flow from bottom to top as: 1. LayoutText falls back to LayoutObject::mapToVisualRectInAncestorSpace() which flips into LayoutRubyText 2. LayoutRubyText uses LayoutBox::mapToVisualRectInAncestorSpace() which flips into LayoutRubyRun 3. LayoutRubyRun uses LayoutBox::mapToVisualRectInAncestorSpace() which flips into LayoutBlockFlow {DIV} as the "local containing block" 4. LayoutRuby (inline) uses LayoutInline::mapToVisualRectInAncestorSpace() which, before this change, would flip again into LayoutBlockFlow {DIV} as the "container" So I believe #4 produces an unnecessary double-flip into the containing DIV. My interpretation of the comment I referenced earlier in the change: https://cs.chromium.org/chromium/src/third_party/WebKit/Source/core/layout/li... // Treat the leading on the first and last lines of ruby runs as not being part of the overall lineTop/lineBottom. // Really this is a workaround hack for the fact that ruby should have been done as line layout and not done using // inline-block. is that our whole implementation of everything underneath perhaps the topmost-level LayoutRuby inline block element representing the <ruby>...</ruby> element would, from perspective of that comment author, have been better modelled as LayoutLine/Inline{Flow,Text}Box-derived logic but, for whatever reason, it was not. I expect one could reasonably argue either side but I'm not (yet?) a line layout expert. I reiterate this just to clarify that I think what we have here is less an inline-block-general issue, and rather something specific to how our implementation contorted/structured things. I will explore some inline, nested span, and inline-block cases locally to see if I turn up anything unusual but I expect if there were bugs they'd already have led to other failing LayoutTests since those cases are fairly common.
	5 </div>

	6 <script>

	7 // Tests that ruby text paint invalidation in flipped-blocks writing mode works

	8 // correctly. The result should be vertical text as a pink "b" with a smaller

	9 // blue "c" to the right of it.

	10 onload = runRepaintTest;

	11 function repaintTest() {

	12 outside.style.color = "pink";

	13 inside.style.color = "blue";

	14 }

	15 </script>

OLD	NEW