Issue metadata
Sign in to add a comment
|
MD Settings: Subpages rendered on top of app toolbar. |
||||||||||||||||||||||
Issue descriptionBisection revealed https://chromium.googlesource.com/chromium/src/+log/e62de92856639b63741c75b7e93e61673d70afc2..8ec0740f51368ce84fd4ad5cd5ba9947f47c96cf I am suspecting r417812, but not sure. Repro steps: 1) Navigate to chrome://md-settings 2) Click "Manage search engines" 3) Scroll down (make sure that the window is short enough). @khushalsagar: Does this bug seem related to the CL changes?
,
Sep 16 2016
Just verified with a revert, I can still repro after that. +lazyboy who had the other change in the bisect.
,
Sep 16 2016
I must have done an error in my first bisection (on the last step). I performed it again and got a different range (which makes more sense), https://chromium.googlesource.com/chromium/src/+log/8ec0740f51368ce84fd4ad5cd5ba9947f47c96cf..8358491511e7af9eb7360e31253bdc610216d59a. From those I verified that r417813 is the culprit. Sorry for the noise.
,
Sep 16 2016
,
Sep 19 2016
+stevenjb: Looking at the culprit CL, this seems also related to iron-lists.
,
Sep 19 2016
So, I am unable to always reproduce this reliably. However, when I do see it, I also see it in the Displays UI which does not use iron-list. (Which doesn't mean that it isn't related). When I revert r417813 I can still reproduce this.
,
Sep 19 2016
I can reproduce this at least as far back as: commit af87b07b5995039753bef996beb9dffc89479be3 Author: chrome-cron <chrome-cron@google.com> Date: Sun Sep 4 20:12:30 2016 Updating trunk VERSION from 2850.0 to 2851.0 Cr-Commit-Position: refs/heads/master@{#416490}
,
Sep 19 2016
@stevenjb: You are correct, it turns out the test I was using to perform the bisection was not 100% deterministic. The issue only happens reliably if the user visits chrome://md-settings/searchEngines directly, not via clicking. Following the updated repro steps, the new bisection range is https://chromium.googlesource.com/chromium/src/+log/540399d5f34abfcae1828e2c36835ffae391e319..d748f22c6fa57ce44370686d816500f8cc9cb2aa and r415440 seems suspect. I have not yet verified by reverting it locally though.
,
Sep 19 2016
I confirmed that r415440 is in fact the culprit: 2ef7df4 CompositingInputsUpdater should do more clip parent analysis by trchen No idea whether this is Polymer specific, but if it is affecting us, it seems likely that it will affect others.
,
Sep 19 2016
,
Sep 20 2016
I was able to successfully revert r415440 with a recent checkout and confirm that the problem is fix, can we go ahead and revert that CL?
,
Sep 20 2016
I don't think reverting is a "fix". With developer tools, you can toggle overflow:hidden on <div id="card"> under #shadow-root of <settings-section page-title="Privacy and security">. The rendering artifact will no longer appear. If you went back and enter the same section again, then the bug appears again. It is a strong evidence that somewhere else in the compositing system using a dirty value for compositing decision, and we should fix that.
,
Sep 20 2016
That seems fine as long as we can get a fix in a reasonable timeframe :) Would explicitly setting overflow:hidden address this for us as a workaround in the near term?
,
Sep 21 2016
Adding a z-index:3 on the cr-toolbar, seems to fix the issue. I don't know why yet, but maybe a potential workaround?
,
Sep 22 2016
Applying overflow:visible on <div id="card"> would be the easiest workaround. I have a fix for this particular case, uploaded for review. It is extremely difficult to implement "clip escaping" 100% correctly under current architecture though.
,
Sep 22 2016
FYI https://codereview.chromium.org/2366683002 is fixing this case (and another bug at the same time).
,
Sep 22 2016
Ignore previous comment. It does not fix both issues unfortunately...
,
Sep 23 2016
Regarding comment#12: Can we re-consider reverting r415440? The current behavior also seems broken, see screencast, where just unchecking and re-checking the z-index in the devtools magically fixes the issue.
,
Sep 23 2016
trchen@: is there anybody else we could talk to about this? you mentioned this might be a bad interaction with compositing. this is a bad bug that's making our page look super broken. we can't just remove the overflow: hidden; as you've suggested, it breaks other things.
,
Sep 23 2016
You might wanna talk to pdr/chrishtr if it's a thing with compositing reasons.
,
Sep 26 2016
Issue 642632 has been merged into this issue.
,
Sep 26 2016
,
Sep 28 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/1de92f8c7df73309f3f192c14cb6755ddea380b5 commit 1de92f8c7df73309f3f192c14cb6755ddea380b5 Author: trchen <trchen@chromium.org> Date: Wed Sep 28 00:14:33 2016 Reset clip parent in CompositedLayerMapping::updateClipParent Reset clip parent instead of skipping update clip parent when owningLayerClippedByLayerNotAboveCompositedAncestor(scrollParent) returns true. This avoids having stale clip parent when the above-mentioned value switched from false to true. Having stale clip parent result in inconsistent rendering comparing to a static page that has a true value since beginning. Technically it is still incorrect but fixing every case of clip escaping is out of the scope of this CL. BUG= 647508 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2 Review-Url: https://codereview.chromium.org/2350363004 Cr-Commit-Position: refs/heads/master@{#421395} [modify] https://crrev.com/1de92f8c7df73309f3f192c14cb6755ddea380b5/third_party/WebKit/LayoutTests/FlagExpectations/enable-slimming-paint-v2 [add] https://crrev.com/1de92f8c7df73309f3f192c14cb6755ddea380b5/third_party/WebKit/LayoutTests/compositing/overflow/clip-parent-reset-expected.html [add] https://crrev.com/1de92f8c7df73309f3f192c14cb6755ddea380b5/third_party/WebKit/LayoutTests/compositing/overflow/clip-parent-reset.html [modify] https://crrev.com/1de92f8c7df73309f3f192c14cb6755ddea380b5/third_party/WebKit/Source/core/layout/compositing/CompositedLayerMapping.cpp
,
Sep 29 2016
,
Sep 29 2016
Is everything working now then? I was planning to spend a little time helping out if needed, per comment 20, but if it's not needed even better.
,
Oct 6 2016
Issue 647167 has been merged into this issue.
,
Oct 10 2016
#25: yes, #23 seems to have fixed our issue. |
|||||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||||
Comment 1 by khushals...@chromium.org
, Sep 16 2016