New issue
Advanced search Search tips

Issue 715758 link

Starred by 1 user

Issue metadata

Status: WontFix
Owner:
Closed: Apr 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 3
Type: Bug



Sign in to add a comment

ScrollTree::MaxScrollOffset doesn't take into account bounds delta on outer viewport

Project Member Reported by bokan@chromium.org, Apr 26 2017

Issue description

This method has a block that looks like this:

if (scroll_node->scrolls_inner_viewport) {
    scroll_bounds.Enlarge(
        property_trees()->inner_viewport_scroll_bounds_delta().x(),
        property_trees()->inner_viewport_scroll_bounds_delta().y());
}

Which applies the bounds delta caused by the URL bar showing/hiding to the clip size so that we correctly calculate the maximum offset. I believe there should be a case here for the outer viewport as well.

This was moved here from LayerImpl::MaxScrollOffset in https://crrev.com/1675963002. In LayerImpl, the method just applied whatever bounds delta was on set on the LayerImpl. Since the outer scroll layer also gets bounds delta applied, I believe we need this branch here.
 

Comment 1 by bokan@chromium.org, Apr 26 2017

Cc: sunxd@chromium.org ajuma@chromium.org
+ajuma@,sunxd@: just wanted to confirm that this wasn't an intentional omission?

Comment 2 by ajuma@chromium.org, Apr 26 2017

The outer viewport scroll layer doesn't currently ever get bounds delta values. See LayerImpl::SetViewportBoundsDelta, which asserts that only the inner viewport scroll layer, or the inner/outer viewport container layers get bounds delta.

Comment 3 by bokan@chromium.org, Apr 26 2017

Status: WontFix (was: Assigned)
Right, sorry, I mixed myself up. And I just found the other bounds deltas are both applied correctly in scroll_clip_layer_bounds(). Thanks for setting me straight :)

Sign in to add a comment