All usage of parentNode() inside ComputedStyleCSSValueMapping.cpp needs to use LayoutTreeBuilderTraversal::parent instead. |
||||||||||
Issue description
The parent you inherit style from is the one computed by the LayoutTreeBuilderTraversal (the flat tree), not the parentNode().
ex.
case CSSPropertyMinHeight:
if (style.minHeight().isAuto()) {
Node* parent = styledNode->parentNode();
if (isFlexOrGrid(parent ? parent->ensureComputedStyle() : nullptr))
return CSSIdentifierValue::create(CSSValueAuto);
return zoomAdjustedPixelValue(0, style);
}
If you have:
<section>
#shadow-root
<div>
and you do getComputedStyle(div).minHeight the parentNode is the ShadowRoot and you'll get the wrong answer.
The same situation comes from slots:
<section>
#shadow-root
<div style="display: flex">
<slot name="inside"></slot>
</div>
<div id="outside" slot="inside"></div>
getComputedStyle(outside).minHeight will compute the parentNode to be the <section> but it should find the <div style="display: flex"> instead.
,
Jan 24 2017
,
Jan 24 2017
This isn't just interop, our current behavior is actually wrong.
,
Jan 25 2017
+ the squad team who may have an idea of where this is happening.
,
Jan 25 2017
Yup only seems to be happening on CSSPropertyMin(Height/Width) ... Seems like a GoodFirstBug. I'll own it and work on it. :)
,
Feb 12 2017
,
Apr 18 2017
,
May 10 2017
Will work on this sometime next week along with other smaller bugs I have been assigned.
,
Jun 14 2017
No progress at the moment.
,
Jul 19 2017
Taken a look at the code and changing the callsite here: https://cs.chromium.org/chromium/src/third_party/WebKit/Source/core/css/ComputedStyleCSSValueMapping.cpp?type=cs&q=%22case+CSSPropertyMinHeight%22&sq=package:chromium&l=2760 to call LayoutTreeBuilderTraversal::Parent(styled_node) returns a nullptr consistently. Investigating further. Test case I am using is here: <section id="section"> <div id="outside" slot="inside"></div> </section> <script> var shadow = section.createShadowRoot(); shadow.innerHTML = "<div style='display: flex'><slot name='inside'></slot></div>" console.log(getComputedStyle(outside).minHeight) </script>
,
Dec 6 2017
,
Jan 31 2018
Marking this as available. I am dropping all non P0/1 issues due to transitioning out of Chrome
,
Mar 5 2018
https://chromium-review.googlesource.com/c/chromium/src/+/948908
,
Mar 5 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/23e4f2edd85fc9f02f6c2c1372b79e48292aca1d commit 23e4f2edd85fc9f02f6c2c1372b79e48292aca1d Author: Rune Lillesveen <futhark@chromium.org> Date: Mon Mar 05 19:46:17 2018 Computed min-width/height for auto depends on box. Resolved value of min-width and min-height for auto min sizing of flex and grid items may be 'auto'. We based this on the computed style of the shadow including parent. Instead we should rely on whether the element will actually be a rendered flex/grid item. Firefox Nightly passes all added tests. The mentioned bug fixed is actually about using the shadow including parent for finding parent computed style instead of the flat tree parent, but that is no longer relevant as we look at the layout tree. Bug: 682869 Change-Id: I6e47dc264a8cdbee7fc1983ce452baf7c0df85b3 Reviewed-on: https://chromium-review.googlesource.com/948908 Reviewed-by: Morten Stenshorne <mstensho@chromium.org> Commit-Queue: Rune Lillesveen <futhark@chromium.org> Cr-Commit-Position: refs/heads/master@{#540901} [add] https://crrev.com/23e4f2edd85fc9f02f6c2c1372b79e48292aca1d/third_party/WebKit/LayoutTests/external/wpt/css/css-flexbox/getcomputedstyle/flexbox_computedstyle_min-auto-size.html [add] https://crrev.com/23e4f2edd85fc9f02f6c2c1372b79e48292aca1d/third_party/WebKit/LayoutTests/external/wpt/css/css-grid/grid-items/grid-item-min-auto-size-001.html [modify] https://crrev.com/23e4f2edd85fc9f02f6c2c1372b79e48292aca1d/third_party/WebKit/Source/core/css/properties/ComputedStyleUtils.cpp
,
Mar 5 2018
|
||||||||||
►
Sign in to add a comment |
||||||||||
Comment 1 by esprehn@chromium.org
, Jan 19 2017