1225508 - about:firefox is truncated with APZ enabled

Status: RESOLVED FIXED

(Core :: Layout, defect)

Description

[Kartikaya Gupta (email:[e-mail])]

If APZ is enabled on Fennec then about:firefox is truncated.

Comment 1

[Kartikaya Gupta (email:[e-mail])]

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

Comment 2

[Kartikaya Gupta (email:[e-mail])]

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.

Comment 3

[Kartikaya Gupta (email:[e-mail])]

Attachment: My bad attempt at fixing it in layout

This was my attempt at trying to fix it in layout, but it didn't work

Comment 4

[Kartikaya Gupta (email:[e-mail])]

Attachment: Patch

This one does work, and is better than a straight backout of bug 1198839

Comment 5

[Kartikaya Gupta (email:[e-mail])]

Comment on attachment 8689088 [details] [diff] [review]
Patch

tn might have other suggestions when he comes online, but I think this is probably ok as well.

Comment 6

[Botond Ballo [:botond]]

Comment on attachment 8689088 [details] [diff] [review]
Patch

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.

Comment 7

[Kartikaya Gupta (email:[e-mail])]

tn, thoughts on how to do this properly in layout?

Comment 8

[Timothy Nikkel (: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.

Comment 9

[Kartikaya Gupta (email:[e-mail])]

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.

Comment 10

[Kartikaya Gupta (email:[e-mail])]

(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 11

[Kartikaya Gupta (email:[e-mail])]

Comment on attachment 8689088 [details] [diff] [review]
Patch

Any objections to landing the workaround for now?

Comment 12

[Botond Ballo [:botond]]

Comment on attachment 8689088 [details] [diff] [review]
Patch

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).

Comment 13

[Kartikaya Gupta (email:[e-mail])]

Thanks! I filed bug 1228602 as a follow-up to fix it properly.

Comment 14

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/92109de6d1a5

Comment 15

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/92109de6d1a5