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

Unified 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 side-by-side diff with in-line comments

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 »
Expand Comments ('e') | Collapse Comments ('c') | Hide Comments ('s')

Index: third_party/WebKit/LayoutTests/paint/invalidation/ruby-text-flipped-blocks.html

diff --git a/third_party/WebKit/LayoutTests/paint/invalidation/ruby-text-flipped-blocks.html b/third_party/WebKit/LayoutTests/paint/invalidation/ruby-text-flipped-blocks.html

new file mode 100644

index 0000000000000000000000000000000000000000..4b2a3d9319af40b0ba23b76ffc7412b2dcb10dcc

--- /dev/null

+++ b/third_party/WebKit/LayoutTests/paint/invalidation/ruby-text-flipped-blocks.html

@@ -0,0 +1,15 @@

+<!doctype html>

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

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

+ <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

wkorman 2016/07/15 01:59:23 I think all of the general cases are fine. The pro

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.

+</div>

+<script>

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

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

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

+onload = runRepaintTest;

+function repaintTest() {

+ outside.style.color = "pink";

+ inside.style.color = "blue";

+</script>