Closed Bug 942995 Opened 6 years ago Closed 6 years ago

APZC for overflow:hidden element should not allow panning

Categories

(Core :: Panning and Zooming, defect)

ARM
Gonk (Firefox OS)
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla28
blocking-b2g 1.3+
Tracking Status
firefox28 --- fixed

People

(Reporter: botond, Assigned: botond)

References

Details

Attachments

(1 file, 1 obsolete file)

The RecordFrameMetrics call here:

http://mxr.mozilla.org/mozilla-central/source/layout/base/nsDisplayList.cpp#1186

unconditionally assigns a non-null scroll id to the root scroll frame of the root content document. With APZ enabled, this causes an APZC to be created for this frame.

If the element corresponding to the root scroll frame is overflow:hidden, this allows the user to scroll the overflow:hidden element by panning. This is undesirable.

Since there is no use in having APZC for an overflow:hidden element, we can solve this problem by getting rid of it in such a case (by assigning NULL_SCROLL_ID to the frame).
blocking-b2g: --- → 1.3+
Attached patch bug942995.patch (obsolete) — Splinter Review
Attachment #8338094 - Flags: review?(tnikkel)
Attachment #8338094 - Flags: review?(tnikkel) → review+
After further discussion (see bug 940691), we don't want to go the route of not having an APZC for an overflow:hidden element, because we always want to have a root APZC. Instead, an APZC for an overflow:hidden element should disallow panning.
Summary: APZC wrongly created for overflow:hidden root scroll frame → APZC for overflow:hidden element should not allow panning
Attached patch bug942995.patchSplinter Review
Reposting Kats' patch from bug 940691. Timothy, can you give it another round of feedback in light of our IRC conversation?
Attachment #8338094 - Attachment is obsolete: true
Attachment #8339494 - Flags: feedback?(tnikkel)
Comment on attachment 8339494 [details] [diff] [review]
bug942995.patch

This seems fine based on what I know now.
Attachment #8339494 - Flags: feedback?(tnikkel) → feedback+
Comment on attachment 8339494 [details] [diff] [review]
bug942995.patch

(In reply to Botond Ballo [:botond] from comment #5)
> Try push: https://tbpl.mozilla.org/?tree=Try&rev=6c920312c805 (also covers
> bug 942799).

Looks clean. Flagging for review.
Attachment #8339494 - Flags: review?(tnikkel)
This one, together with the patch for bug 942799 takes care of most of the first wave of problems with APZC that I've seen.  The only one that I still see is in the gallery app (bug 944511)
Attachment #8339494 - Flags: review?(tnikkel) → review+
https://hg.mozilla.org/mozilla-central/rev/2139b34164d9
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla28
Depends on: 945260
This broke touch scrolling in metro for top level frames. If there isn't an easy fix, we should back this out. Unfortunately it made it into this morning's nightly.
Flags: needinfo?(nhirata.bugzilla)
Depends on: 946340
Blocks: 950488
No longer blocks: 950488
Flags: needinfo?(nhirata.bugzilla)
You need to log in before you can comment on or make changes to this bug.