One more...I was looking into:
compositing/layer-creation/stacking-context-overlap-nested.html
and found that when a box has outline, visual overflow calls AddOutlineRects, but since its semantics is a bit different from legacy, it produces different layer bounds. Legacy includes calls AddOutlineRects only for inline, and exclude OOF. NG calls for all boxes, and include OOF. Don't know whether this difference is ignorable or we should match to legacy...
The following revision refers to this bug:
https://chromium.googlesource.com/chromium/src.git/+/57e65e7ea0ddff1a848bebe3aed5dd4ba1e6a1aa
commit 57e65e7ea0ddff1a848bebe3aed5dd4ba1e6a1aa
Author: Aleks Totic <atotic@chromium.org>
Date: Mon Sep 17 07:16:34 2018
Reimplement LayoutInline::VisualRectInDocument
Existing implementation of LayoutInline::VisualRectInDocument is
broken for Continuations. I've discovered this while implementing
outlines for LayoutNG.
The test case that failed was
fast/spatial-navigation/snav-div-overflow-scroll-hidden.html
Relevant html is:
<a href="#" id="f6"><img src="resources/green.png" width=50px; height=40px;></a>
<br>
<a href="#" id="f7"><img src="resources/green.png" width=40px; height=40px;></a>
Layout tree
LayoutNGBlockFlow (anonymous) 0x101355c250b8
LayoutInline 0x101355c34850 A id="f6" (focused)
LayoutText 0x101355c4c470 #text "\n "
LayoutBR 0x101355c540e0 BR
=>LayoutInline 0x101355c34910 continuation=0x101355c25720 A id="f7"
LayoutNGBlockFlow (anonymous) 0x101355c25720 continuation=0x101355c349d0
LayoutImage 0x101355c40700 IMG
LayoutNGBlockFlow (anonymous) 0x101355c25200
LayoutInline 0x101355c349d0 A id="f7"
LayoutText 0x101355c4c550 #text "\n "
LayoutInline 0x101355c34a90 continuation=0x101355c25868 A id="f8"
VisualRectInDocument was called on f7, 0x101355c34910
In the original code, visual rect is computed by traversing
all the anonymous blocks above. It would have added the visual rects
from all of the:
LayoutInline 0x101355c34850 A id="f6" (focused)
LayoutText 0x101355c4c470 #text "\n "
LayoutBR 0x101355c540e0 BR
LayoutImage 0x101355c40700 IMG
This is obviously incorrect. Correct traversal should have added:
LayoutInline 0x101355c34910 continuation=0x101355c25720 A id="f7"
LayoutImage 0x101355c40700 IMG
LayoutInline 0x101355c349d0 A id="f7"
The traversal that should happen is exactly the same traversal
AddOutlineRects does.
I replaced existing code with AddOutlineRects, and no test failed.
Bug: 835484
Change-Id: I8ced39163aaec2a1d3c46a8776837b58ea966e4a
Reviewed-on: https://chromium-review.googlesource.com/1226367
Commit-Queue: Aleks Totic <atotic@chromium.org>
Reviewed-by: Morten Stenshorne <mstensho@chromium.org>
Reviewed-by: Xianzhu Wang <wangxianzhu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#591616}
[modify] https://crrev.com/57e65e7ea0ddff1a848bebe3aed5dd4ba1e6a1aa/third_party/blink/renderer/core/layout/layout_inline.cc
[modify] https://crrev.com/57e65e7ea0ddff1a848bebe3aed5dd4ba1e6a1aa/third_party/blink/renderer/core/layout/layout_inline.h
[modify] https://crrev.com/57e65e7ea0ddff1a848bebe3aed5dd4ba1e6a1aa/third_party/blink/renderer/core/layout/layout_inline_test.cc
Comment 1 by e...@chromium.org
, Apr 23 2018Status: Assigned (was: Untriaged)