Closed Bug 1082594 Opened 5 years ago Closed 5 years ago

Inactive subframes are not added to the dispatch-to-content region

Categories

(Core :: Layout, defect)

All
macOS
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla36

People

(Reporter: kats, Assigned: dvander)

References

Details

Attachments

(1 file, 1 obsolete file)

As part of the contract of bug 928833 (if I understand correctly), if you have a page with an iframe that is not actively scrolled, the bounds of that iframe should be included in the dispatch-to-content region of the layer. This will allow the APZ code to pass input events through to content, which will then set a displayport on the iframe and make it actively scrolled in its own layer. However, this doesn't appear to be happening.

STR:
- Start Firefox on OS X with a clean profile. Enable the layers.dump and layout.event-regions.enabled prefs from about:config. This will dump the layers tree including the event regions to stdout
- Load a page like http://people.mozilla.org/~kgupta/gridframe.html where there is an iframe
- Observe the layers dump - there are no dispatchtocontent regions anywhere in the layer tree. I expect the PaintedLayer for the content to include a dispatchtocontent region that includes the bounds of the iframe.
roc, as I continue working on bug 918288 I'm likely to uncover a bunch of bugs like these. I'd like to have a clear ownership process for who will be working on these bugs, as they are on the critical path for our B2G 2.2 features. Thoughts?
Flags: needinfo?(roc)
(In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #0)
> As part of the contract of bug 928833 (if I understand correctly), if you
> have a page with an iframe that is not actively scrolled, the bounds of that
> iframe should be included in the dispatch-to-content region of the layer.
> This will allow the APZ code to pass input events through to content, which
> will then set a displayport on the iframe and make it actively scrolled in
> its own layer.

Right. The problem is that code to add inactive scrollports to the dispatch-to-content region has not been added.

(In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #1)
> roc, as I continue working on bug 918288 I'm likely to uncover a bunch of
> bugs like these. I'd like to have a clear ownership process for who will be
> working on these bugs, as they are on the critical path for our B2G 2.2
> features. Thoughts?

Maybe Timothy can take these bugs?
Flags: needinfo?(roc) → needinfo?(tnikkel)
Assignee: nobody → dvander
Status: NEW → ASSIGNED
Attachment #8523389 - Flags: review?(tnikkel)
Comment on attachment 8523389 [details] [diff] [review]
bug1082594-inactive-scrollframe-dispatch.patch

Until bug 1009786 lands and unifies IsScrollingActive with getting an async scrollable layer I think we will need to use a different test to determine when to add the scrollable area to the dispatch to content region. I think the condition you want is mShouldBuildScrollableLayer being false and shouldBuildLayer being true. This means that it is an async scrollable frame, but we aren't creating an async scrollable layer for it (yet).
Flags: needinfo?(tnikkel)
Address comment #4. This still works, but it runs afoul of another bug that I'll file shortly.
Attachment #8523389 - Attachment is obsolete: true
Attachment #8523389 - Flags: review?(tnikkel)
Attachment #8524963 - Flags: review?(tnikkel)
Comment on attachment 8524963 [details] [diff] [review]
bug1082594-inactive-scrollframe-dispatch.patch

I think we still need to rely on scroll info layers until not only this bug is fixed, but also layer event regions are enabled, and I think we'll need some more pieces to get the dispatch to content region solution to create a display port and start an async scroll. So r+ on the non-APZCTreeManager parts. I'll let kats weigh in on that.
Attachment #8524963 - Flags: review?(tnikkel)
Attachment #8524963 - Flags: review?(bugmail.mozilla)
Attachment #8524963 - Flags: review+
Comment on attachment 8524963 [details] [diff] [review]
bug1082594-inactive-scrollframe-dispatch.patch

Review of attachment 8524963 [details] [diff] [review]:
-----------------------------------------------------------------

APZ side looks fine. The code being modified is all conditional upon the event-regions pref being enabled and so it shouldn't affect the event-regions-disabled case.
Attachment #8524963 - Flags: review?(bugmail.mozilla) → review+
https://hg.mozilla.org/mozilla-central/rev/19b303e7e7f2
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla36
You need to log in before you can comment on or make changes to this bug.