about:firefox is truncated with APZ enabled

RESOLVED FIXED in Firefox 45



2 years ago
a year ago


(Reporter: kats, Assigned: kats)


Dependency tree / graph

Firefox Tracking Flags

(firefox45 fixed)



(2 attachments)

If APZ is enabled on Fennec then about:firefox is truncated.
Assignee: nobody → bugmail.mozilla
So the problem here is that the RCD doesn't have a displayport on it, but it does have a resolution. (And more specifically, the resolution is < 1). The content has this meta tag:

width=480; initial-scale=.6667; user-scalable=no

Since the content fits in the 480 CSS pixel width (and corresponding height), it is non-scrollable and we don't put a displayport on it. The code at [1] also skips putting margins on it because APZ hasn't set a displayport on it.

However, without the displayport, layout doesn't take into account the resolution, so it only paints the area of the page that would be visible without the resolution. This results in the truncation.

[1] https://dxr.mozilla.org/mozilla-central/rev/a2f83cbe53ac4009afa4cb2b0b8f549289b23eeb/layout/base/MobileViewportManager.cpp#218
Component: Graphics, Panning and Zooming → Layout
Product: Firefox for Android → Core
One fix here is to just back out bug 1198839, but that's probably undesirable in the general case. The "correct" fix is probably in layout, to take into account the resolution when computing the dirty rect of things. I'll see if I can write that up.
Created attachment 8689076 [details] [diff] [review]
My bad attempt at fixing it in layout

This was my attempt at trying to fix it in layout, but it didn't work
Created attachment 8689088 [details] [diff] [review]

This one does work, and is better than a straight backout of bug 1198839
Comment on attachment 8689088 [details] [diff] [review]

tn might have other suggestions when he comes online, but I think this is probably ok as well.
Attachment #8689088 - Flags: review?(botond)
Comment on attachment 8689088 [details] [diff] [review]

Review of attachment 8689088 [details] [diff] [review]:

This sounds reasonable as a workaround, but I'd like to hear from Timothy about whether we can solve the problem in Layout, which feels like the more proper solution.
Attachment #8689088 - Flags: review?(botond) → feedback+
tn, thoughts on how to do this properly in layout?
Flags: needinfo?(tnikkel)
Hmm, is it just getting the dirty rect right that we need to support resolution without display port fully?

That might not be too bad, I can take a look. The setting a displayport workaround also seems reasonable to me.
I was assuming it was just the dirty rect that needed to be corrected. When I looked at the layer dump the visible region on the PaintedLayer was wrong (i.e. it was too small), and there might be other things in layout that need to be updated as well.
(In reply to Timothy Nikkel (:tn) from comment #9)
> This part
> https://hg.mozilla.org/integration/mozilla-inbound/rev/0aa2e79b9252#l4.13
> of bug 1224015 appears to have fixed bug 1225508 for me. This makes sense to
> me. (I tested with and without that hunk.) So I think you are good to land
> without the workaround.

I'm still seeing this issue. Note that it only manifests when the about:firefox page is NOT scrollable by default. For me, on a try or nightly build, about:firefox IS scrollable (because there's a big "Check for Updates" button) but a local build doesn't have that button and so the page is just short enough that it's not scrollable. So on a local build I see the truncation problem but not on a Nightly. My observations are specific to the Nexus 4; devices with taller screens will probably have the issue on Nightly builds as well, because the page won't be scrollable. Devices with shorter screens may never see this problem.
Comment on attachment 8689088 [details] [diff] [review]

Any objections to landing the workaround for now?
Attachment #8689088 - Flags: feedback+ → review?(botond)
Comment on attachment 8689088 [details] [diff] [review]

Review of attachment 8689088 [details] [diff] [review]:

No objection, as long as we're tracking the proper solution somewhere (either in a follow-up bug, or by leaving this bug open).
Attachment #8689088 - Flags: review?(botond) → review+
Depends on: 1228602
Thanks! I filed bug 1228602 as a follow-up to fix it properly.
Flags: needinfo?(tnikkel)

Comment 14

2 years ago

Comment 15

2 years ago
Last Resolved: 2 years ago
status-firefox45: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla45
See Also: → bug 1243514
You need to log in before you can comment on or make changes to this bug.