Bug 121889

Summary:

[Windows] Attempting to scroll from a non-scrollable layer results the main document getting scrolled

Product:

WebKit

Reporter:

Roger Fong <roger_fong>

Component:

WebKit Misc.

Assignee:

Nobody <webkit-unassigned>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

bdakin, bfulgham, commit-queue, esprehn+autocc, glenn, hyatt, jonlee, kondapallykalyan, roger_fong, simon.fraser, thorton

Priority:

Keywords:

InRadar

Version:

528+ (Nightly build)

Hardware:

Unspecified

OS:

Unspecified

Attachments:

Description	Flags
patch	none
patch	thorton: review+

Roger Fong

Reported 2013-09-24 19:07:56 PDT

This applies to anything that uses RenderLayer::scrollbyRecursive to scroll and where the initial caller of the method is a layer that is not scrollable and has no overflow clip. The win port uses this for scrolling in response to a panning gesture. If a layer that for which hasOverflowCiip returns false is first touched when intiating the pan we end up just scrolling the main document. This is because the method doesn't appropriately recursively search up the RenderLayer tree. It should walk up the RenderLayer tree in its entirety and not decide to just bail as soon as we reach a layer that has no overflow clip.

Attachments
patch (3.13 KB, patch) 2013-09-25 15:16 PDT, Roger Fong	no flags	Details Formatted Diff Diff
patch (1.52 KB, patch) 2013-09-25 16:01 PDT, Roger Fong	thorton: review+	Details Formatted Diff Diff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.

Roger Fong

Comment 1 2013-09-24 19:08:52 PDT

<rdar://problem/15072441>

Roger Fong

Comment 2 2013-09-24 19:16:21 PDT

I'm not sure why we haven't seen any issues on mac...hrmmm...will have to look through usages of scrollbyrecursive in the mac port.

Roger Fong

Comment 3 2013-09-24 19:21:33 PDT

(In reply to comment #2) > I'm not sure why we haven't seen any issues on mac...hrmmm...will have to look through usages of scrollbyrecursive in the mac port. O! The win port is calling this method from a layer returned by "RenderLayer::enclosingLayer()" instead of "RenderLayer::enclosingScrollableLayer()", that might be the real issue.

Roger Fong

Comment 4 2013-09-25 14:55:48 PDT

I could make scrollbyrecursively calling itself on its enclosingScrollableLayer if the layer itself is not scrollable. However, it seems the pattern here in general is that whatever calls this method is already a scrollable layer, so I will change the win port to match that for now. I think we should consider fixing this method in the future though. Will add a fixme.

Roger Fong

Comment 5 2013-09-25 15:16:22 PDT

Created attachment 212630 [details] patch

Roger Fong

Comment 6 2013-09-25 16:01:12 PDT

Created attachment 212632 [details] patch removed the fixme, filed a separate bug for the issue: https://bugs.webkit.org/show_bug.cgi?id=121929

Tim Horton

Comment 7 2013-09-25 16:04:35 PDT

The changelog could be better.

Roger Fong

Comment 8 2013-09-25 16:24:48 PDT

Committed: http://trac.webkit.org/changeset/156433 with a better changelog

Tim Horton

Comment 9 2013-09-25 16:27:02 PDT

(In reply to comment #8) > Committed: http://trac.webkit.org/changeset/156433 > with a better changelog agreed

Note You need to log in before you can comment on or make changes to this bug.