1053975 - Trivial fullscreen app gets incorrect viewport on Flame (contentScale=1.5)

Status: RESOLVED FIXED

(Core :: Graphics: Layers, defect)

Description

[Benoit Girard (:BenWa)]

Attachment: app.zip

+++ This bug was initially created as a clone of Bug #1052751 +++

The attached app is a simple body { background: red; } with no meta viewport tag.

I believe the app should either be width 918 or have the width of the screen. However I'm seeing that the viewport is, when converted to device pixels, off by 1.5x.

I/Gecko   ( 1198):         RefLayerComposite (0xb0107400) [shadow-clip=(x=0, y=0, w=480, h=854)] [shadow-visible=< (x=0, y=0, w=480, h=854); >] [clip=(x=0, y=0, w=480, h=854)] [visible=< (x=0, y=0, w=480, h=854); >] [id=4]
I/Gecko   ( 1198):           ContainerLayerComposite (0xabdbfc00) [shadow-transform=[ 0.444444 0; 0 0.444444; 0 0; ]] [shadow-visible=< (x=0, y=0, w=480, h=854); >] [postScale=1.5, 1.5] [transform=[ 0.666667 0; 0 0.666667; 0 0; ]] [visible=< (x=0, y=0, w=480, h=854); >] [opaqueContent] [metrics={ viewport=(x=0.000000, y=0.000000, w=480.000000, h=854.000000) cb=(x=0.000000, y=0.000000, w=480.000000, h=854.000000) viewportScroll=(x=0.000000, y=0.000000) displayport=(x=0.000000, y=0.000000, w=480.000000, h=854.000000) critdp=(x=0.000000, y=0.000000, w=480.000000, h=854.000000) scrollableRect=(x=0.000000, y=0.000000, w=480.000000, h=854.000000) scrollId=3 }] [preScale=1.5, 1.5]

The viewport is w=480 but it should be 480/1.5 since it's in CSS Pixels. I wonder if we don't take the content scale into account when we hit this patch.

Comment 1

[Botond Ballo [:botond]]

First, let's clarify what viewport width we expect: we expect 320 (480 / 1.5), not 980, because in bug 940036, we added special handling for apps that fail to specify a meta viewport tag, treating them as if they had "width=device-width, height=device-height, user-scalable=no".

Now, let's see why we're getting 480 instead of 320:

  - nsDocument::GetViewportInfo() constructs the nsViewportInfo object
    for an app with no meta viewport tag here [1], passing in the
    screen size as an ScreenSize, which (correctly) has width=480

  - nsViewportInfo stores the size as a CSSSize. To convert from the
    ScreenSize to the CSSSize, it divides by 'mDefaultZoom', which is
    a CSSToScreenScale, here [2].

  - CSSToScreenScale is supposed to include the device scale, which is
    1.5 on a Flame.

  - However, the nsViewportInfo constructor used at [1] initializes
    mDefaultZoom to 1.0. As a result, the CSSSize that is calculated
    and stored has width=480 (incorrectly).

[1] http://mxr.mozilla.org/mozilla-central/source/content/base/src/nsDocument.cpp#7537
[2] http://mxr.mozilla.org/mozilla-central/source/content/base/public/nsViewportInfo.h#35

Comment 2

[Botond Ballo [:botond]]

Attachment: Fix

Try push that includes this patch: https://tbpl.mozilla.org/?tree=Try&rev=0eed380c2ead

Comment 3

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

The fix seems sound, but can you rebase this patch on top of bug 974242? mbrubeck should review this as well.

Comment 4

[Botond Ballo [:botond]]

(In reply to Botond Ballo [:botond] from comment #2)
> Try push that includes this patch: https://tbpl.mozilla.org/?tree=Try&rev=0eed380c2ead

The build failures are not related to this patch :) (That's what I get for trying to combine Try pushes.)

Comment 5

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

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

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

Clearing review for now; I can review the rebased version.

Comment 6

[Botond Ballo [:botond]]

Attachment: Fix

Comment 7

[Botond Ballo [:botond]]

Rebased as requested. Marking bug as dependent on bug 974242 accordingly.

Comment 8

[Botond Ballo [:botond]]

Trying again now that bug 974242 has landed: https://tbpl.mozilla.org/?tree=Try&rev=99c64793844d

Comment 9

[Botond Ballo [:botond]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/68d28e7fb162

Comment 10

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

https://hg.mozilla.org/mozilla-central/rev/68d28e7fb162