757362 - Swizzle toolbar icons don't load

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Robert Nyman]

The top icons on the right hand side at http://theswizzle.com/ don't load in Aurora (base64 images through CSS).

Tested in Aurora on Galaxy Nexus.

Comment 1

[Joe Drew (not getting mail)]

Explicitly renoming. We want to find out what's causing this; it's possible that it's their layout, in which case we can just minus this, but if we're doing something wrong, we should fix it.

Martijn, can you minimize this testcase?

Comment 2

[Aaron Train [:aaronmt]]

Still reproducible on Nightly (20120523).

Comment 3

[Martijn Wargers (dead)]

Attachment: testcase

On the Galaxy Nexus, the float: right; div is invisible with width: 258px; in portrait mode, but becomes visible with width: 259px;.

On the Galaxy SII and HTC Desire HD, only the width: 500px; div is visible in portrait mode.

In landscape mode, every div is visible, except on the Galaxy SII and HTC Desire HD, the bottom block isn't shown at all there for some reason.

Comment 4

[Martijn Wargers (dead)]

This isn't a problem in XUL Fennec, so a regression range might be useful here.

Comment 5

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

First guess is that this has to do with the clip rect on position:fixed elements, and therefore may be related to bug 759389. I'll investigate further.

Comment 6

[Naoki Hirata :nhirata (please use needinfo instead of cc)]

icon loads on the nightly 3/5/2012 build
icon loads on the nightly 4/17/2012 build
icon loads on the nightly 5/7/2012 build

Looks like a recent regression.  still looking for regression range (need to go to meeting)

Comment 7

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

The clip rect is definitely wrong; I'm seeing the clip rect getting scaled down from what it should be. On my even-more-simplifed test page at http://people.mozilla.org/~kgupta/bug/757362.html (assumes a device of 720 pixel width such as the GN) I get the following layer dump info.

24691560[5edb1080]: BasicLayerManager (0x5e80f820)
24691560[5edb1080]:   BasicContainerLayer (0x5da27820) [visible=< (x=0, y=0, w=980, h=1396); >] [opaqueContent] [metrics={ viewport=(x=0, y=0, w=980, h=1396) viewportScroll=(x=0, y=0) displayport=(x=0, y=0, w=980, h=1396) scrollId=1 }]
24691560[5edb1080]:     BasicThebesLayer (0x65e8fe60) [clip=(x=0, y=0, w=0, h=0)]
24691560[5edb1080]:     BasicColorLayer (0x5e328140) [clip=(x=0, y=0, w=980, h=1396)] [visible=< (x=0, y=0, w=980, h=1396); >] [opaqueContent] [color=rgba(255, 255, 255, 1)]
24691560[5edb1080]:     BasicThebesLayer (0x65e8fff0) [visible=< (x=719, y=0, w=104, h=50); (x=720, y=50, w=104, h=50); (x=719, y=100, w=104, h=50); >] [opaqueContent] [isFixedPosition]

The rects for the three divs are in the last BasicThebesLayer and seem to have the correct position and size. Then there is:

27111648[63f84be0]: OGLLayerManager (0x63f7b200)
27111648[63f84be0]:   OGLShadowContainerLayer (0x64004890) [shadow-transform=[ 1.36111 0; 0 1.36111; 0 0; ]] [shadow-visible=< (x=0, y=0, w=529, h=754); >] [transform=[ 1.36111 0; 0 1.36111; 0 0; ]] [visible=< (x=0, y=0, w=529, h=754); >] [metrics={ viewport=(x=0, y=0, w=529, h=754) viewportScroll=(x=0, y=0) displayport=(x=0, y=0, w=0, h=0) scrollId=0 }]
27111648[63f84be0]:     OGLShadowThebesLayer (0x64004c90) [opaqueContent] [isFixedPosition]
27111648[63f84be0]:     OGLShadowContainerLayer (0x65e95c90) [shadow-clip=(x=0, y=0, w=529, h=754)] [shadow-transform=[ 1 0; 0 1; 0 0; ]] [shadow-visible=< (x=0, y=0, w=530, h=755); >] [clip=(x=0, y=0, w=529, h=754)] [transform=[ 1 0; 0 1; 0 0; ]] [visible=< (x=0, y=0, w=530, h=755); >] [isFixedPosition]
27111648[63f84be0]:       OGLShadowContainerLayer (0x65e96090) [shadow-transform=[ 0.734694 0; 0 0.734694; 0 0; ]] [shadow-visible=< (x=0, y=0, w=720, h=1026); >] [transform=[ 1 0; 0 1; 0 0; ]] [visible=< (x=0, y=0, w=720, h=1026); >] [metrics={ viewport=(x=0, y=0, w=720, h=1026) viewportScroll=(x=0, y=0) displayport=(x=0, y=0, w=720, h=1026) scrollId=3 }]
27111648[63f84be0]:         OGLShadowThebesLayer (0x65e96890) [shadow-clip=(x=0, y=0, w=0, h=0)] [clip=(x=0, y=0, w=0, h=0)]
27111648[63f84be0]:         OGLShadowColorLayer (0x643b9250) [shadow-clip=(x=0, y=0, w=720, h=1026)] [shadow-visible=< (x=0, y=0, w=720, h=1026); >] [clip=(x=0, y=0, w=720, h=1026)] [visible=< (x=0, y=0, w=720, h=1026); >] [opaqueContent] [color=rgba(255, 255, 255, 1)]
27111648[63f84be0]:         OGLShadowThebesLayer (0x65e96c90) [shadow-visible=< (x=528, y=0, w=77, h=37); (x=528, y=73, w=77, h=37); >] [visible=< (x=528, y=0, w=77, h=37); (x=528, y=73, w=77, h=37); >] [opaqueContent] [isFixedPosition] [valid=< (x=528, y=0, w=77, h=37); (x=528, y=73, w=77, h=37); >]

where you can see a clip rect that is scaled down to 529x754 in some places. I seem to recall some other bug where this double-scaling was happening but can't find it now. CC'ing ajuma for any insights.

Comment 8

[Naoki Hirata :nhirata (please use needinfo instead of cc)]

    I made a mistake.  I was looking at the wrong thing.

    icons do show in 3/14/2012 build
    icons do not show 3/15/2012 build

    http://hg/mozilla.org/mozilla-central/rev/d0d13f09be44
    http://hg.mozilla.org/mozilla-central/shortlog/d0d13f09be44

Comment 9

[Ali Juma [:ajuma]]

The clip that's 529x754 is on the second container layer. Its child container layer has a 0.734694 scaling shadow transform applied though, so the clip seems fine.

Comment 10

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

blocking-1.0 bug update: I'm continuing to investigate this bug. I'm not yet sure if it's a layout or viewport issue. At some point in the layout code there is a viewport rect that is not what it should be so I'm trying to figure out where it's coming from. Assistance from anybody who knows that area of the code better would be appreciated.

Comment 11

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

Ok, so disregard what I said in comment 7. I updated the test case at http://people.mozilla.org/~kgupta/bug/757362.html and ran it through again and got some slightly different results. During the building of the display list, it seems like the dirty rect used for out-of-flow children is smaller than what is visible on the screen. I'm not yet sure if this is actually wrong or not.

There is a call to MarkAbsoluteFramesForDisplayList at this point:

#4  nsIFrame::BuildDisplayListForStackingContext
    at /layout/generic/nsFrame.cpp:1800
#5  nsSubDocumentFrame::BuildDisplayList
    at /layout/generic/nsSubDocumentFrame.cpp:314
#6  nsIFrame::BuildDisplayListForChild
    aFlags=0) at /layout/generic/nsFrame.cpp:2099
#7  nsDeckFrame::BuildDisplayListForChildren
    at /layout/xul/base/src/nsDeckFrame.cpp:162
#8  nsBoxFrame::BuildDisplayList
    at /layout/xul/base/src/nsBoxFrame.cpp:1340
#9  nsDeckFrame::BuildDisplayList
    at /layout/xul/base/src/nsDeckFrame.cpp:146
#10 nsIFrame::BuildDisplayListForChild
    aFlags=0) at /layout/generic/nsFrame.cpp:2099
#11 nsBoxFrame::BuildDisplayListForChildren
    at /layout/xul/base/src/nsBoxFrame.cpp:1378
#12 nsBoxFrame::BuildDisplayList
    at /layout/xul/base/src/nsBoxFrame.cpp:1340
#13 BuildDisplayListWithOverflowClip
    at /layout/generic/nsFrame.cpp:1602
#14 nsIFrame::BuildDisplayListForChild
    aFlags=0) at /layout/generic/nsFrame.cpp:2097
#15 nsBoxFrame::BuildDisplayListForChildren
    at /layout/xul/base/src/nsBoxFrame.cpp:1378
#16 nsRootBoxFrame::BuildDisplayList
    at /layout/xul/base/src/nsRootBoxFrame.cpp:214
#17 nsIFrame::BuildDisplayListForChild
    aFlags=0) at /layout/generic/nsFrame.cpp:2099
#18 ViewportFrame::BuildDisplayList
    at /layout/generic/nsViewportFrame.cpp:71
#19 nsIFrame::BuildDisplayListForStackingContext
    at /layout/generic/nsFrame.cpp:1813
#20 nsLayoutUtils::PaintFrame
    at /layout/base/nsLayoutUtils.cpp:1656
#21 PresShell::Paint
    at /layout/base/nsPresShell.cpp:5241
#22 nsViewManager::Refresh
    at /view/src/nsViewManager.cpp:341

and the dirtyRect that is passed in here is (0, 0, 43200, 61560). The actual area visible on the screen is more like (0, 0, 58797, 83787). The fixed-position element at (43200, 3000, 6000, 3000) doesn't intersect the dirtyRect and therefore is not added to the display list, and hence is never drawn.

Note that the displayport area is picked up later in the display-list-building process, here:

#0  nsGfxScrollFrameInner::BuildDisplayList 
    at /layout/generic/nsGfxScrollFrame.cpp:2238
#1  0x6103d74a in nsHTMLScrollFrame::BuildDisplayList 
    at /layout/generic/nsGfxScrollFrame.h:359
#2  0x61033d74 in nsIFrame::BuildDisplayListForChild 
    aFlags=0) at /layout/generic/nsFrame.cpp:2099
#3  0x6107a394 in ViewportFrame::BuildDisplayList 
    at /layout/generic/nsViewportFrame.cpp:71
#4  0x6102f8c6 in nsIFrame::BuildDisplayListForStackingContext 
    at /layout/generic/nsFrame.cpp:1813
#5  0x61066486 in nsSubDocumentFrame::BuildDisplayList 
    at /layout/generic/nsSubDocumentFrame.cpp:314
#6  0x61033d74 in nsIFrame::BuildDisplayListForChild 
    aFlags=0) at /layout/generic/nsFrame.cpp:2099
#7  0x61125800 in nsDeckFrame::BuildDisplayListForChildren 
    at /layout/xul/base/src/nsDeckFrame.cpp:162
#8  0x61111d6c in nsBoxFrame::BuildDisplayList 
    at /layout/xul/base/src/nsBoxFrame.cpp:1340
#9  0x6112578c in nsDeckFrame::BuildDisplayList 
    at /layout/xul/base/src/nsDeckFrame.cpp:146
#10 0x61033d74 in nsIFrame::BuildDisplayListForChild 
    aFlags=0) at /layout/generic/nsFrame.cpp:2099
#11 0x61111b9a in nsBoxFrame::BuildDisplayListForChildren 
    at /layout/xul/base/src/nsBoxFrame.cpp:1378
#12 0x61111d6c in nsBoxFrame::BuildDisplayList 
    at /layout/xul/base/src/nsBoxFrame.cpp:1340
#13 0x610302f2 in BuildDisplayListWithOverflowClip 
    at /layout/generic/nsFrame.cpp:1602
#14 0x61033d62 in nsIFrame::BuildDisplayListForChild 
    aFlags=0) at /layout/generic/nsFrame.cpp:2097
#15 0x61111b9a in nsBoxFrame::BuildDisplayListForChildren 
    at /layout/xul/base/src/nsBoxFrame.cpp:1378
#16 0x6110fc16 in nsRootBoxFrame::BuildDisplayList 
    at /layout/xul/base/src/nsRootBoxFrame.cpp:214
#17 0x61033d74 in nsIFrame::BuildDisplayListForChild 
    aFlags=0) at /layout/generic/nsFrame.cpp:2099
#18 0x6107a394 in ViewportFrame::BuildDisplayList 
    at /layout/generic/nsViewportFrame.cpp:71
#19 0x6102f8c6 in nsIFrame::BuildDisplayListForStackingContext 
    at /layout/generic/nsFrame.cpp:1813
#20 0x60fd60d4 in nsLayoutUtils::PaintFrame 
    at /layout/base/nsLayoutUtils.cpp:1656
#21 0x60feb128 in PresShell::Paint 
    at /layout/base/nsPresShell.cpp:5241
#22 0x613457c2 in nsViewManager::Refresh 
    at /view/src/nsViewManager.cpp:341

The displayport is correct - (0, 0, 58797, 83787) - and this is used as the dirty rect for all non-out-of-flow elements inside the nsGfxScrollFrame. If it were used for the out-of-flow elements as well then presumably the green box would show up. Again, I'm not clear on what the expected behaviour is for fixed position elements so I don't know what needs to be corrected here.

Comment 12

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

Re-asking my question from https://bugzilla.mozilla.org/show_bug.cgi?id=607417#c226 here and CC'ing roc as blassey suggested so that it doesn't get lost and also has better context for an answer.

My question is: what is the current expected behaviour for position:fixed elements that are not at 0,0? On this test page, there is a position:fixed element at left:720 on a page that has a 980-pixel wide CSS viewport. When viewing this page on a 720-pixel wide device, the element isn't rendered at all.

There is another position:fixed element at left:719 which is entirely visible at the default zoom (~0.73). As I zoom in it moves farther to the right on the screen, and behaves like a normal position:fixed element in that it's screen coordinates don't change as I pan around. At a zoom of ~1.0 it goes offscreen and is no longer visible, and cannot be seen even when I pan. This means I can never see this element at a high zoom level. Is this the expected behaviour?

Comment 13

[Brad Lassey [:blassey] (use needinfo?)]

Kats, renom if you think this should block after roc answers your question.

Comment 14

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Layout always positions position:fixed elements relative to the CSS viewport. However, CompositorParent then manipulates their layers and I don't really understand that code.

Comment 15

[Chris Lord [:cwiiis]]

(In reply to Kartikaya Gupta (:kats) from comment #12)
> Re-asking my question from
> https://bugzilla.mozilla.org/show_bug.cgi?id=607417#c226 here and CC'ing roc
> as blassey suggested so that it doesn't get lost and also has better context
> for an answer.
> 
> My question is: what is the current expected behaviour for position:fixed
> elements that are not at 0,0? On this test page, there is a position:fixed
> element at left:720 on a page that has a 980-pixel wide CSS viewport. When
> viewing this page on a 720-pixel wide device, the element isn't rendered at
> all.
> 
> There is another position:fixed element at left:719 which is entirely
> visible at the default zoom (~0.73). As I zoom in it moves farther to the
> right on the screen, and behaves like a normal position:fixed element in
> that it's screen coordinates don't change as I pan around. At a zoom of ~1.0
> it goes offscreen and is no longer visible, and cannot be seen even when I
> pan. This means I can never see this element at a high zoom level. Is this
> the expected behaviour?

This is indeed expected behaviour, for the moment. The code in CompositorParent only handles the situation where the compositor-side scroll position is different to the content-side scroll position (so this bug exists, regardless of the work in bug 607417).

This problem is bug 760805, which will be fixed in bug 758620.

Comment 16

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

Thanks for the responses. Marking this dependent on bug 758620, and leaving as minused for blocking-1.0 fennec as it looks like it will be a significant amount of work to fix.

There are probably hacks I could do to fix this in the short term. One that comes to mind is to increase the size of the dirtyRect passed to MarkAbsoluteFramesForDisplayList for the nsSubdocumentFrame (or just ignore the dirtyRect entirely and just mark all out-of-flow children as needing paint). However this seems like it will regress performance and possibly also regress correctness in other ways, so I would like to not do that.

Comment 17

[Timothy Nikkel (:tnikkel)]

Attachment: patch

In BuildDisplayListForStackingContext when we mark absolute (fixed) frames we use the normal dirty rect and not the displayport, so we don't get the right fixed items in the display list.

Comment 18

[Timothy Nikkel (:tnikkel)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/5ba4c3a2338c

Comment 19

[Ryan VanderMeulen [:RyanVM]]

FWIW, this was backed out and relanded for suspected OSX reftest orange. Sorry for the churn.
https://hg.mozilla.org/integration/mozilla-inbound/rev/4a3cdbee92e8
https://hg.mozilla.org/integration/mozilla-inbound/rev/1eb62e8e0946

Comment 20

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/1eb62e8e0946

Comment 21

[Timothy Nikkel (:tnikkel)]

Do we want this on aurora?

Comment 22

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

It's tracking fennec 15+ so yes, it should be nominated for aurora uplift.

Comment 23

[Timothy Nikkel (:tnikkel)]

Comment on attachment 635053 [details] [diff] [review]
patch

[Approval Request Comment]
Bug caused by (feature/regressing bug #): been around for a while
User impact if declined: some fixed position elements won't be drawn
Testing completed (on m-c, etc.): baked on m-c for several days
Risk to taking this patch (and alternatives if risky): too too risky
String or UUID changes made by this patch: none

Comment 24

[Alex Keybl [:akeybl]]

(In reply to Timothy Nikkel (:tn) from comment #23)
> Risk to taking this patch (and alternatives if risky): too too risky

Not too risky? Just want to be sure :)

Comment 25

[Timothy Nikkel (:tnikkel)]

(In reply to Alex Keybl [:akeybl] from comment #24)
> (In reply to Timothy Nikkel (:tn) from comment #23)
> > Risk to taking this patch (and alternatives if risky): too too risky
> 
> Not too risky? Just want to be sure :)

Oops! Yes I meant "not too risky".

Comment 26

[Lukas Blakk [:lsblakk] use ?needinfo]

Comment on attachment 635053 [details] [diff] [review]
patch

approving since it's not too risky :)

Comment 27

[Timothy Nikkel (:tnikkel)]

https://hg.mozilla.org/releases/mozilla-aurora/rev/cbf349be82f1

Comment 29

[Paul Feher]

Verified/fixed on:
Aurora Fennec 15.0a2 (2012-07-08)
Using:
HTC Desire Z (2.3.3)

The issue in the bug is not reproducible. The test case in the bug works correctly. The duplicated  bug 770857 is no longer reproducible.