[Webkit-unassigned] [Bug 198788] REGRESSION(r245818): Async scrolling: Fixed position banner jumps around page

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=198788

Simon Fraser (smfr) <simon.fraser at apple.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #371955|review?                     |review+
              Flags|                            |

--- Comment #3 from Simon Fraser (smfr) <simon.fraser at apple.com> ---
Comment on attachment 371955
  --> https://bugs.webkit.org/attachment.cgi?id=371955
patch

View in context: https://bugs.webkit.org/attachment.cgi?id=371955&action=review

> Source/WebCore/ChangeLog:3
> +        REGRESSION(r245818): Async scrolling: Fixed position banner jumps around page

Change the title to make it clear this is about fixed inside sticky.

> Source/WebCore/page/scrolling/cocoa/ScrollingTreeFixedNode.mm:106
> +            if (is<ScrollingTreePositionedNode>(*ancestor)) {
> +                auto& positioningAncestor = downcast<ScrollingTreePositionedNode>(*ancestor);
> +                // See if sticky node already handled this positioning node.
> +                // FIXME: Include positioning node information to sticky/fixed node to avoid these tests.
> +                if (lastStickyNode && lastStickyNode->layer() == positioningAncestor.layer())
> +                    continue;
> +                if (positioningAncestor.layer() != m_layer)
> +                    overflowScrollDelta -= positioningAncestor.scrollDeltaSinceLastCommit();
> +                continue;
> +            }
> +
> +            if (is<ScrollingTreeStickyNode>(*ancestor)) {
> +                auto& stickyNode = downcast<ScrollingTreeStickyNode>(*ancestor);
> +                overflowScrollDelta += stickyNode.scrollDeltaSinceLastCommit();
> +                lastStickyNode = &stickyNode;
> +                continue;
> +            }

It seems weird for ScrollingTreeFixedNode to have to test all its ancestor node types. This was the problem that the old delta was trying to solve, I think; these ancestors should pass state down the tree for the applyLayerPositions() tree walk.

> Source/WebCore/rendering/RenderLayerCompositor.cpp:3057
> +        if (ancestor->hasCompositedScrollableOverflow())
> +            return true;

But this might not be in the ancestor paint order chain. A simple fixed -> non-stacking overflow -> fixed should not need to composite the inner fixed. I think we only need to composite the inner fixed if some paint-order ancestor has a scrolling tree node (but we can't do that check at this stage).

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-unassigned/attachments/20190612/3e81bdbe/attachment.html>