New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 814141 link

Starred by 5 users

Issue metadata

Status: Fixed
Owner:
Closed: Aug 30
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows , Mac
Pri: 2
Type: Bug



Sign in to add a comment

position:sticky doesn't pay attention to scroller padding when scroller is composited

Reported by wart.cl...@gmail.com, Feb 21 2018

Issue description

UserAgent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_13_3) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/64.0.3282.167 Safari/537.36

Steps to reproduce the problem:
1. scroll down until .sticky becomes sticky
2. check top spacing

What is the expected behavior?
sticky element should be 30px from top. It should take padding into account when determining the top value.

What went wrong?
sticky element is 0 from top.

This behaviour is different then Chrome for windows, Safari and Firefox.

Did this work before? N/A 

Does this work in other browsers? Yes

Chrome version: 64.0.3282.167  Channel: stable
OS Version: OS X 10.13.3
Flash Version: 

also tested this in Chrome Canary (66.0.3350.0) and the issue still exists.
 
sticky-chrome.html
2.7 KB View Download
When testing at the office, it seems that it happens at some devices, but not all.

We tested on 8 different macOS devices, all with the same Chrome version and 4 people have the expected behaviour and 4 don't. We all have the exact same user agent string so that can't be the issue.
Labels: Needs-Triage-M64

Comment 3 by e...@chromium.org, Feb 22 2018

Components: -Blink>CSS Blink>Input
Cc: vamshi.kommuri@chromium.org
Labels: Triaged-ET M-66 FoundIn-66 Target-66
Status: Untriaged (was: Unconfirmed)
Thanks for filing the issue!

Able to reproduce the issue on reported Chrome version  64.0.3282.167 and on the latest canary 65.0.3352.0 using Mac 10.13.1. As the issue is seen from M60(60.0.3072.0) considering it as Non-Regression and marking it as Untriaged.
Note: Issue is not seen in Windows 10 and Ubuntu 14.04.
Components: -Blink>Input Internals>Compositing>Animation

Comment 6 by sunxd@chromium.org, Feb 23 2018

Owner: smcgruer@chromium.org
Status: Assigned (was: Untriaged)
Hi Stephen, can you take a look at this position:sticky bug?
Cc: flackr@chromium.org
Labels: OS-Linux OS-Windows
Status: Started (was: Assigned)
Summary: position:sticky doesn't pay attention to padding when scroller is composited (was: position:sticky uses wrong top on MacOS)
It's because some of your devices are low-DPI (where we don't composite the scroller due to losing LCD text) and some are high-DPI (where we will basically always composite it).

Note that for whatever reason, the important point is whether the scroller is composited, not the sticky element. For some reason we're ignoring padding for the composited side.
Summary: position:sticky doesn't pay attention to scroller padding when scroller is composited (was: position:sticky doesn't pay attention to padding when scroller is composited)
Minimal repro attached.


sticky.html
461 bytes View Download

Comment 9 by flackr@chromium.org, Feb 26 2018

Looks like on the blink side we offset the clip rect:
https://cs.chromium.org/chromium/src/third_party/WebKit/Source/core/layout/LayoutBoxModelObject.cpp?type=cs&sq=package:chromium&l=1029

But on cc we just use the content rect:
https://cs.chromium.org/chromium/src/cc/trees/property_tree.cc?type=cs&q=file:cc+stickypositionoffset&sq=package:chromium&l=377

We should add the border and padding to the constraints so that the calculation code is identical in cc / blink
Components: -Internals>Compositing>Animation Internals>Compositing>Scroll
Status: Assigned (was: Started)
Though I know the sticky section of the position spec is far from actually defined and you are going for interop  more or less, I'm interested in hearing from implementors about...

How padding is understood for a sticky positioned elements containing block when that containing block comes from the scrolling element versus the containing block coming from an element inside the scrolling element that has padding?

In the former, your approach makes the padding compound the box offset value. The latter, padding has no affect. When in both scenarios the space for the padding box moves through the scrolling element while scrolling?

Could anybody explain to me why this is the case?
Also, notice how if the scrolling element is the html or body elements then padding does NOT contribute to the computed sticking position of a sticky elements box offset between all vendors?
Status: Started (was: Assigned)
Cc: trchen@chromium.org
 Issue 877098  has been merged into this issue.
Project Member

Comment 16 by bugdroid1@chromium.org, Aug 30

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/56559ffeae48bd3903e7a62ed03e42e795ab9fac

commit 56559ffeae48bd3903e7a62ed03e42e795ab9fac
Author: Stephen McGruer <smcgruer@chromium.org>
Date: Thu Aug 30 17:22:32 2018

position:sticky - respect scroller padding when composited

This CL fixes the cc-side logic for computing the clip rect to
pay attention to the scroller padding. Unlike in Blink we do
not need to adjust for the border, since it isn't included in the
cc-side scroll bounds[0].

Tests are added for both scroller padding and border; the latter is
important as per crbug.com/753124 we may someday change the cc bounds
to include the border. If/when that happens we will need to offset
for the border size as well.

[0]: https://cs.chromium.org/chromium/src/third_party/blink/renderer/platform/graphics/compositing/paint_artifact_compositor.cc?l=203&rcl=3ad6465f517124e8a23434c21f1c543ad47b1d5a

Bug:  814141 
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_layout_tests_slimming_paint_v2;master.tryserver.blink:linux_trusty_blink_rel
Change-Id: Ie5371198fee3710150c24d077fc31a7da1d037d2
Reviewed-on: https://chromium-review.googlesource.com/1180071
Reviewed-by: Philip Rogers <pdr@chromium.org>
Reviewed-by: Tien-Ren Chen <trchen@chromium.org>
Reviewed-by: Yi Gu <yigu@chromium.org>
Commit-Queue: Stephen McGruer <smcgruer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#587638}
[modify] https://crrev.com/56559ffeae48bd3903e7a62ed03e42e795ab9fac/cc/layers/layer_sticky_position_constraint.cc
[modify] https://crrev.com/56559ffeae48bd3903e7a62ed03e42e795ab9fac/cc/layers/layer_sticky_position_constraint.h
[modify] https://crrev.com/56559ffeae48bd3903e7a62ed03e42e795ab9fac/cc/trees/layer_tree_host_common_unittest.cc
[modify] https://crrev.com/56559ffeae48bd3903e7a62ed03e42e795ab9fac/cc/trees/property_tree.cc
[add] https://crrev.com/56559ffeae48bd3903e7a62ed03e42e795ab9fac/third_party/WebKit/LayoutTests/compositing/overflow/composited-sticky-overflow-border-expected.html
[add] https://crrev.com/56559ffeae48bd3903e7a62ed03e42e795ab9fac/third_party/WebKit/LayoutTests/compositing/overflow/composited-sticky-overflow-border.html
[add] https://crrev.com/56559ffeae48bd3903e7a62ed03e42e795ab9fac/third_party/WebKit/LayoutTests/compositing/overflow/composited-sticky-overflow-padding-expected.html
[add] https://crrev.com/56559ffeae48bd3903e7a62ed03e42e795ab9fac/third_party/WebKit/LayoutTests/compositing/overflow/composited-sticky-overflow-padding.html
[modify] https://crrev.com/56559ffeae48bd3903e7a62ed03e42e795ab9fac/third_party/blink/renderer/core/layout/layout_box_model_object.cc
[modify] https://crrev.com/56559ffeae48bd3903e7a62ed03e42e795ab9fac/third_party/blink/renderer/core/layout/layout_box_model_object_test.cc
[modify] https://crrev.com/56559ffeae48bd3903e7a62ed03e42e795ab9fac/third_party/blink/renderer/core/paint/compositing/composited_layer_mapping.cc

Status: Fixed (was: Started)
Closing as fixed. Re comments #12 and #13, please file issues at https://github.com/w3c/csswg-drafts/labels/css-position-3 to discuss sticky spec questions. Thanks! :)
Cc: yigu@chromium.org rkalavakuntla@chromium.org
 Issue 790943  has been merged into this issue.

Sign in to add a comment