Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(658)

Issue 1172153004: Absolute-position the initial containing block. (Closed)

Created:
5 years, 6 months ago by skobes
Modified:
5 years, 6 months ago
Reviewers:
esprehn
CC:
darktears, apavlov+blink_chromium.org, blink-reviews, blink-reviews-css, blink-reviews-style_chromium.org, dglazkov+blink, rwlbuis, szager1
Base URL:
https://chromium.googlesource.com/chromium/blink.git@master
Target Ref:
refs/heads/master
Project:
blink
Visibility:
Public.

Description

Absolute-position the initial containing block. LayoutView needs to clip its contents when root layer scrolling is enabled. It already does, but only as a side effect of being composited, and some things hit-test in front of the scrollbars because DeprecatedPaintLayerClipper thinks absolute-positioned content can only be clipped by positioned containers. Generally a static-positioned scroller will not clip absolute-positioned descendants, so this seems like the cleanest fix (versus special-casing LayoutView in DPLClipper). I think there are other cases where the assumption in DPLClipper may be wrong (in particular, when the scroller has a transform), but that can be addressed separately; see http://crbug.com/499083. Two test cases in WebFrameTest no longer pass with root layer scrolling, but they only worked by accident. Fix tracked in http://crbug.com/499053. BUG=495856 Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=197054

Patch Set 1 : #

Unified diffs Side-by-side diffs Delta from patch set Stats (+36 lines, -6 lines) Patch
A LayoutTests/fast/scrolling/absolute-position-behind-scrollbar.html View 1 chunk +23 lines, -0 lines 0 comments Download
A LayoutTests/fast/scrolling/absolute-position-behind-scrollbar-expected.txt View 1 chunk +5 lines, -0 lines 0 comments Download
M Source/core/css/resolver/StyleResolver.cpp View 1 chunk +1 line, -0 lines 0 comments Download
M Source/core/layout/LayoutObject.cpp View 1 chunk +2 lines, -1 line 0 comments Download
M Source/core/layout/TextAutosizer.cpp View 1 chunk +1 line, -1 line 0 comments Download
M Source/web/tests/WebFrameTest.cpp View 3 chunks +4 lines, -4 lines 0 comments Download

Messages

Total messages: 12 (7 generated)
skobes
5 years, 6 months ago (2015-06-11 00:37:08 UTC) #7
esprehn
lgtm, does the spec say this somewhere?
5 years, 6 months ago (2015-06-12 06:16:55 UTC) #8
skobes
On 2015/06/12 06:16:55, esprehn wrote: > lgtm, does the spec say this somewhere? As far ...
5 years, 6 months ago (2015-06-12 18:37:37 UTC) #9
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1172153004/100001
5 years, 6 months ago (2015-06-12 18:37:58 UTC) #11
commit-bot: I haz the power
5 years, 6 months ago (2015-06-12 19:32:53 UTC) #12
Message was sent while issue was closed.
Committed patchset #1 (id:100001) as
https://src.chromium.org/viewvc/blink?view=rev&revision=197054

Powered by Google App Engine
This is Rietveld 408576698