936277 - Subframe displayports sometimes get clipped away

Status: RESOLVED FIXED

(Core :: Layout, defect)

Description

[Botond Ballo [:botond]]

Issue (1) https://bugzilla.mozilla.org/show_bug.cgi?id=912666#c25 turns out to be a general layout issue, not something specific to the dynamic toolbar work, though it affects the dynamic toolbar implementation in a very visible way.

When a display list is built for a scroll frame, the scroll frame applies a clip to the scrolled content. If the scroll frame does not have a display port, the clip is the size of the scroll port; otherwise, it's the size of the display port. [1]

The clip is applied to the scrolled content by adding into a piece of state storing the in the display list builder called the "clip state". When child display items are created, they initialize their clip rects from the clip state.

If there is a pre-existing clip state when the scroll frame wants to set a clip, the clip state is updated to be the intersection of the two. This gives rise to the following situation:

If you have an outer scroll frame without a display port and an inner scroll frame with a display port, the outer scroll frame will set the clip state to its scroll port; the inner scroll frame will then update the clip state to the intersection of its own display port and the previous clip state, which is the outer frame's scroll port. This can have the effect of clipping away most or all of the intended display port. (When it comes to painting, regions that have been clipped away are not painted, so for a display port to serve its intended purpose, it's important that it doesn't get clipped away.)

Importantly, this doesn't happen if the outer scroll frame is a root scroll frame, because in such a case, ScrollFrameHelper::BuildDIsplayList() exits early at [2], and does not set a clip on its children.

So, to sum up, this only affects cases where:
  - There is an inner scroll frame with a display port.
  - There is an outer scroll frame which
      - is not a root scroll frame
      - does not have a display port

I guess this situation doesn't happen often and so this issue hasn't been noticed so far. It does happen with my dynamic toolbar implementation, though, so I would like to fix it.

The proposed fix is to ignore any pre-existing clip state when a scroll frame sets a clip on its scrolled content, and only use the scroll frame's scroll port or display port.

Thanks to :tn for helping me debug this issue and proposing a fix!

[1] http://mxr.mozilla.org/mozilla-central/source/layout/generic/nsGfxScrollFrame.cpp#2312
[2] http://mxr.mozilla.org/mozilla-central/source/layout/generic/nsGfxScrollFrame.cpp#2256

Comment 1

[Botond Ballo [:botond]]

Attachment: Fix

Comment 2

[Botond Ballo [:botond]]

The patch resolves issue (1) from https://bugzilla.mozilla.org/show_bug.cgi?id=912666#c25.

Comment 3

[Botond Ballo [:botond]]

Try run: https://tbpl.mozilla.org/?tree=Try&rev=f66f4593bd26

Comment 4

[Botond Ballo [:botond]]

(In reply to Botond Ballo [:botond] from comment #3)
> Try run: https://tbpl.mozilla.org/?tree=Try&rev=f66f4593bd26

Try run is clean.

Comment 5

[Timothy Nikkel (:tnikkel)]

Comment on attachment 828988 [details] [diff] [review]
Fix

Can you add a comment saying that the content will indeed be clipped to the scrollport, just lower down when we build the scroll wrapper?

Comment 6

[Botond Ballo [:botond]]

Attachment: bug936277.patch

Added comment as per tn's suggestion. Carrying r+.

Comment 7

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/57882166c5d4