Fieldset in custom element's shadow dom blocks inheritable CSS properties in nested custom elements
Reported by
philip.m...@gmail.com,
Mar 13 2018
|
||||||||
Issue descriptionUserAgent: Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/65.0.3325.146 Safari/537.36 Steps to reproduce the problem: 1. Create custom element #1 with a shadow dom 2. In this element's shadow dom, add the structure: "fieldset > div > slot" 3. Create custom element #2 that renders content in a shadow dom 4. Custom element #2 should use some inherited CSS properties that are intended to pierce the shadow dom (font, custom properties, etc). 5. Wrap custom element #2 with custom element #1 (so the that #2 is rendered via #1's slot. What is the expected behavior? CSS properties that should be inherited (such as font-family) and CSS custom properties (CSS variables) should be applied. What went wrong? CSS properties that should be inherited are not applied. Replacing the fieldset element with any other element resolves the problem. Did this work before? Yes 64 Does this work in other browsers? Yes Chrome version: 65.0.3325.146 Channel: stable OS Version: Flash Version: I realize the test case is a little complex, I've tried to simplify as much as I can see possible. This is a production issue for us where we are wrapping Polymer's paper-radio-buttons in a fieldset that is part of another custom element - although we are releasing a workaround which removes the fieldset and thus fixes the issue. The test case I uploaded can also be found in this jsbin: http://jsbin.com/mimizok/edit?html,css,js,console,output
,
Mar 14 2018
Able to reproduce the issue on reported chrome version 65.0.3325.146 latest stable 65.0.3325.162 and latest beta 65.0.3325.124 using Windows 10, Ubuntu 14.04 and Mac 10.13.1. The issue is not seen on latest canary 67.0.3369.0 hence providing reverse bisect info. Reverse Bisect information: =========================== Last Bad Build : 67.0.3368.0 First Good build: 67.0.3369.0 You are probably looking for a change made after 542469 (known good), but no later than 542470 (first known bad). CHANGELOG URL: https://chromium.googlesource.com/chromium/src/+log/00e1b9bc8f88495a5ce98419bb352b902758ad9c..bddb211b216ed6844cb64a7ec51b069e0ac044b5 Suspecting: https://chromium.googlesource.com/chromium/src/+/bddb211b216ed6844cb64a7ec51b069e0ac044b5 Review URL: https://chromium-review.googlesource.com/958465 @Rune Lillesveen: Please help in assigning it to others if this is not related to your change. Additional info- Normal bisect range where it has broken earlier in M65: ---------------------------------------------- Last Good Build: 65.0.3284.0 First Bad Build: 65.0.3285.0 Note: Adding RB-Stable as this is a recent regression, please feel free to remove if not applicable. Thanks!
,
Mar 14 2018
This probably broke with https://crrev.com/b71f5b7df345f1a8603b7cf7b59edf48288e7d96 We stopped recalculating re-attachment style for elements with custom style hooks. If that happens in a shadow tree, we will end up with shadow tree descendants with no computed style, possibly down to a slot. Slotted elements (host children) are recalculated afterwards and we didn't check if slotted element parent computed style was computed before computing style for the slotted element, ending up with initial style as the style from which we are inheriting. What's special with fieldset here is that it has custom style hooks. Other elements with custom style hooks will have the same issue.
,
Mar 14 2018
The fix for this is indeed: https://crrev.com/bddb211b216ed6844cb64a7ec51b069e0ac044b5 It's fixed by the fact that we are checking for parent computed style before calculating computed style as part of the style recalc pass.
,
Mar 14 2018
This bug requires manual review: Request affecting a post-stable build Please contact the milestone owner if you have questions. Owners: cmasso@(Android), cmasso@(iOS), bhthompson@(ChromeOS), govind@(Desktop) For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Mar 14 2018
Has this been tested in Canary?
,
Mar 14 2018
Also could you pls justify M65 merge? How critical and safe it is to consider for M65 respin? Please not M65 is already in stable so merge bar is VERY high.
,
Mar 15 2018
Let's not merge for M65, then.
,
Mar 15 2018
Re Canary, it went into 67.0.3369.0 according to chromiumdash.
,
Mar 15 2018
Your change meets the bar and is auto-approved for M66. Please go ahead and merge the CL to branch 3359 manually. Please contact milestone owner if you have questions. Owners: cmasso@(Android), cmasso@(iOS), josafat@(ChromeOS), abdulsyed@(Desktop) For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Mar 15 2018
Rejecting merge to M65 branch based on comment #9. Thank you.
,
Mar 15 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/62221431022ae38f900136df249d1923efb67506 commit 62221431022ae38f900136df249d1923efb67506 Author: Rune Lillesveen <futhark@chromium.org> Date: Thu Mar 15 15:37:20 2018 [Squad] Don't compute style for attaching non-flat-tree elements. RecalcStyleForReattach would compute style for slot elements even though they were not part of the flat tree before SlotInFlatTree runtime flag was introduced. Moved SetNonAttachedStyle(nullptr) to Element to make sure we only ever set or clear non-attached style on Elements, and added DCHECKs to support that and the fact that we only set non-attached style on elements which are part of the flat tree. This was partly the regression for 792080, but will not have an effect for the performance tests on master since SlotInFlatTree is enabled. Bug: 792080 , 821361 Change-Id: I47659300dea9cf4411a4e06f574dbad15c29fdfb Reviewed-on: https://chromium-review.googlesource.com/958465 Reviewed-by: Morten Stenshorne <mstensho@chromium.org> Commit-Queue: Rune Lillesveen <futhark@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#542470}(cherry picked from commit bddb211b216ed6844cb64a7ec51b069e0ac044b5) Reviewed-on: https://chromium-review.googlesource.com/964501 Reviewed-by: Rune Lillesveen <futhark@chromium.org> Cr-Commit-Position: refs/branch-heads/3359@{#269} Cr-Branched-From: 66afc5e5d10127546cc4b98b9117aff588b5e66b-refs/heads/master@{#540276} [modify] https://crrev.com/62221431022ae38f900136df249d1923efb67506/third_party/WebKit/Source/core/dom/Element.cpp [modify] https://crrev.com/62221431022ae38f900136df249d1923efb67506/third_party/WebKit/Source/core/dom/Node.cpp
,
Mar 21 2018
Verified the fix on Mac 10.13.1, Windows-10 and Ubuntu 14.04 using Chrome version #66.0.3359.45 as per the comment #0. Attaching screen shot for reference. Observed that CSS custom properties (CSS variables) are applied properly. Hence, the fix is working as expected. Adding the verified labels. Note: Able to reproduce the issue on chrome version(65.0.3325.146) with out fix. Thanks...!! |
||||||||
►
Sign in to add a comment |
||||||||
Comment 1 by viswa.karala@chromium.org
, Mar 13 2018