Description: When selected they just show up white, but you can still slide them left or right to get to the 1st and 4th images which do show. Repro Steps: 1) Updated Buri 1.3 mozRIL to BuildID: 20140131004001 2) Connect data or wireless 3) Launch Marketplace and search for AccuWeather 4) Select AccuWeather app page and press the second and third preview images Actual: Images do not load and screen is shown white. Expected: Images load and work properly. Environmental Variables: Device: Buri 1.3 MOZ BuildID: 20140131004001 Gaia: 0ddcd8da5bfe1b48c73502ef29220e92f2db6b73 Gecko: 32e45047b663 Version: 28.0a2 Firmware Version: v1.2-device.cfg Notes: The X on the top right to close the unloaded images does not work as well, the user needs to scroll to the first or fourth image to close out of the image preview slideshow. Repro frequency: 100%, 2/2 See attached: http://www.youtube.com/watch?v=iTx2TpxBrq8

This does not repro on 1.2 Environmental Variables: Device: Buri 1.2 MOZ BuildID: 20140128004004 Gaia: 539a25e1887b902b8b25038c547048e691bd97f6 Gecko: d10e1f965d0c Version: 26.0 Firmware Version: v1.2-device.cfg

Not a marketplace bug.

Updated STR: Repro Steps: 1) Updated Buri 1.3 mozRIL to BuildID: 20140131004001 2) Connect data or wireless 3) Launch Marketplace and search/select any app 4) Select any preview images except the first and last Notes: I went through and checked a dozen other apps and it seems that other than the first and last images, no others can be viewed full screen.

Wil, can you take a look if this is marketplace or not? if not, maybe talk to graphics team.

It's hard to tell from the slim info available. Would be great to see requests/responses and Error/JS console. If the images are 404s or there is a JS error, it's our fault, but otherwise I don't think so. I tried to reproduce on desktop and couldn't so my first reaction is that it isn't a Marketplace bug. Let me know if I can be helpful.

qawanted for Wil's requests in comment #5.

This issue started to occur on the Buri 1.3 Build ID: 20140110004009 Environmental Variables: Device: BURI 1.3 MOZ BuildID: 20140110004009 Gaia: c606b129a2d1647c0fc7bfb197555026e9b27f9e Gecko: c5109884ae3a Version: 28.0a2 Firmware Version: V1.2-device.cfg Last working Buri (branch) Build ID: Environmental Variables: Device: BURI 1.3 MOZ BuildID: 20140109004002 Gaia: 22bc6be5b76cdc6d4e9667ff070979041a20ce2f Gecko: 2c8f8683bd0d Version: 28.0a2 Firmware Version: V1.2-device.cfg

This could possibly be related to bug 964517, or not, but I doubt it's ImageLib.

Let's say tentatively that this is APZC, as bug 907179 changes are in the regression range.

Milan, what's the meta bug for all APZC dependencies like these?

Actually this is 1.3+ so i'll make it block gaia-apzc

I've been trying to figure out what's going on here but haven't made much progress. It only happens with APZC enabled. The position:fixed lightbox that shows the screenshots is set to width:100% and uses CSS transforms to display the appropriate one of the four images. The layer tree and displaylist dump seem normal as far as I can tell. If, using app manager, I reduce the width of the lightbox element, the images shrink to match, but still only the first screenful of them are rendered. I thought it might be an issue with the image-visibility prefs but even after hard-coding PresShell::AssumeAllImagesVisible() to return true the issue persists. I'll probably try creating a simplified test case to reproduce this.

Ah, making the lightbox not position:fixed makes all the images render. So definitely related to that.

Here's a simplified test case that reproduces the problem. It works fine on desktop.

Actually after much poking and prodding this looks like a race condition. Even with the simplified test case sometimes it works fine I do really quick taps or longer taps but medium length taps don't work.

I think the timing thing I mentioned above is a bit of a red herring. I think the timing only affects when the displayport gets set on the document, which is an artifact of the reduced test case and shouldn't occur on the full site. However, in the reduced test case, commenting out the displayport-dependent chunk of code in ContainerState::FindFixedPosFrameForLayerData seems to fix the problem. I don't understand that code but I know it has changed between 1.3 and master. I will see if the code that's on master now fixes the problem; if not I'll have to come up with some questions to ask roc to understand the code, or hand it off to somebody else.

(In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #16) > commenting out the > displayport-dependent chunk of code in > ContainerState::FindFixedPosFrameForLayerData seems to fix the problem. Specifically the new visible region that is set in that code seems to be the problem. roc suggested that the fixed-pos code in AsyncCompositionManager.cpp is wrong but I tried commenting out that stuff and it didn't help. If the problem is there then it's something that code should be doing (to the visible region) but isn't. I'm not convinced that's the case.

I still think the problem is in the code at [1]. Specifically I think there's some sort of coordinate system mismatch here. I loaded the simplified test case (attachment 8373681 [details]), and tapped on the text and broke into this function in gdb and looked around. At this point the relevant values are as follows: (gdb) p displayPort $1 = { <mozilla::gfx::BaseRect<int, nsRect, nsPoint, nsSize, nsMargin>> = { x = 0, y = 0, width = 19200, height = 21900 }, <No data fields>} (gdb) p aDrawRegion $2 = (const nsIntRegion &) @0x43fbde60: { mImpl = { mRectCount = 3, mCurRect = 0x43f7619c, mRectListHead = { <nsRegion::nsRectFast> = { <nsRect> = { <mozilla::gfx::BaseRect<int, nsRect, nsPoint, nsSize, nsMargin>> = { x = 2147483647, y = 2147483647, width = 0, height = 0 }, <No data fields>}, <No data fields>}, members of nsRegion::RgnRect: prev = 0x43f7619c, next = 0x43f76334 }, mBoundRect = { <nsRect> = { <mozilla::gfx::BaseRect<int, nsRect, nsPoint, nsSize, nsMargin>> = { x = -1, y = -1, width = 390, height = 42 }, <No data fields>}, <No data fields>} } } (gdb) p *aVisibleRegion $3 = { mImpl = { mRectCount = 1, mCurRect = 0x43f7616c, mRectListHead = { <nsRegion::nsRectFast> = { <nsRect> = { <mozilla::gfx::BaseRect<int, nsRect, nsPoint, nsSize, nsMargin>> = { x = 2147483647, y = 0, width = 0, height = 0 }, <No data fields>}, <No data fields>}, members of nsRegion::RgnRect: prev = 0x43f7616c, next = 0x43f7616c }, mBoundRect = { <nsRect> = { <mozilla::gfx::BaseRect<int, nsRect, nsPoint, nsSize, nsMargin>> = { x = 320, y = 19, width = 69, height = 22 }, <No data fields>}, <No data fields>} } } After the displayport is adjusted by the offset to cross doc, it becomes: (gdb) p displayPort $4 = { <mozilla::gfx::BaseRect<int, nsRect, nsPoint, nsSize, nsMargin>> = { x = -480, y = -1440, width = 19200, height = 21900 }, <No data fields>} These values result in aVisibleRegion getting updated from from (320,19,69,22) to (-1,-1,210,42), which seems wrong to me. I think that in this code the displayport (after adjustment) is in a different coordinate space compared to aDrawRegion. One of these seems to be taking into account the CSS transform and the other does not, and when we "And" them together into the newVisibleRegion it comes out wrong. roc, mattwoodrow, you guys know this code best - any suggestions? I will also attach output that shows the displaylist and layer tree dumps from this run in case it is useful. [1] http://mxr.mozilla.org/mozilla-b2g28_v1_3/source/layout/base/FrameLayerBuilder.cpp?rev=9eaff8e0448d#1642

What's the stacktrace to this happening? As long as we're not inside an inactive layer, the all coordinates should be in post-transform space. The numbers don't look too broken to me. We've made the visible region bigger to include everything that fits within the display port.

The final state of the fixed-pos layer is 02-12 11:11:15.109 I/Gecko ( 522): ClientThebesLayer (0x43598aa0) [transform=[ 1 0; 0 1; 8 8; ]] [visible=< (x=0, y=15, w=69, h=42); >] [opaqueContent] [isFixedPosition anchor=-8.000000,-8.000000 margin=-1.000000,0.000000,0.000000,-1.000000] [valid=< (x=0, y=15, w=209, h=42); >] The visible region (x=0, y=15, w=69, h=42) looks about right to me --- that should be about the dimensions and position of the word "SUCCESS" within the fixed-pos element. But you're saying that in this testcase, the SUCCESS text isn't rendered properly/at all? It seems like we must have failed to paint it to the layer properly. Maybe enable the paint-dumping code in PaintInactiveLayer?

(In reply to Matt Woodrow (:mattwoodrow) from comment #20) > What's the stacktrace to this happening? Attached. > As long as we're not inside an inactive layer, the all coordinates should be > in post-transform space. > > The numbers don't look too broken to me. We've made the visible region > bigger to include everything that fits within the display port. But the visible region is also shifted over from being at x=320 to x=-1, and that's where the problem comes in. If I leave newVisibleRegion at the same size, but shift it back to x=320 y=19 (via newVisibleRegion.MoveBy(321,20)) then the text renders fine. Note that doing this doesn't seem to affect the final state of the fixed-pos layer that roc comments on; the visible region there remains the same. (In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #21) > The final state of the fixed-pos layer is > 02-12 11:11:15.109 I/Gecko ( 522): ClientThebesLayer (0x43598aa0) > [transform=[ 1 0; 0 1; 8 8; ]] [visible=< (x=0, y=15, w=69, h=42); >] > [opaqueContent] [isFixedPosition anchor=-8.000000,-8.000000 > margin=-1.000000,0.000000,0.000000,-1.000000] [valid=< (x=0, y=15, w=209, > h=42); >] > > The visible region (x=0, y=15, w=69, h=42) looks about right to me --- that > should be about the dimensions and position of the word "SUCCESS" within the > fixed-pos element. Yup, that's correct. > But you're saying that in this testcase, the SUCCESS text isn't rendered > properly/at all? Yes, and it seems directly related to the "newVisibleRegion" value computed. > It seems like we must have failed to paint it to the layer > properly. Maybe enable the paint-dumping code in PaintInactiveLayer? Perhaps? When I enable the two sDumpPainting blocks in PaintInactiveLayer, I get the following additional output in logcat: data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAEUAAAAqCAYAAADlL5amAAAAIklEQVRoge3BgQAAAADDoPlTX+AIVQEAAAAAAAAAAAAA8A0tcgABkxHyogAAAABJRU5ErkJggg== That looks pretty empty to me.

roc is debugging this locally, but I think the problem is indeed that we're trying to set fixed position layer data on an inactive ThebesLayer. I think this loop: http://mxr.mozilla.org/mozilla-b2g28_v1_3/source/layout/base/FrameLayerBuilder.cpp#1636 Shouldn't be able to cross the reference frame boundary, otherwise we change coordinate space and it makes no sense.

Yes, that's exactly right.

This is the simple, safe fix. I'll file a followup bug with additional fixes for other issues I found.

Actually, let's take this as well, just to be sure.

Filed bug 972713 for followup.

I rebased these patches onto 1.3 and verified that they fix the original reported problem as well as the simplified test case. I tested part 2 by itself first and then applied part 1 as well - both seemed to be fine.

https://hg.mozilla.org/integration/mozilla-inbound/rev/1cbbc597b7b5 https://hg.mozilla.org/integration/mozilla-inbound/rev/4df228e4fa8b

https://hg.mozilla.org/mozilla-central/rev/1cbbc597b7b5 https://hg.mozilla.org/mozilla-central/rev/4df228e4fa8b

Please request b2g28 approval on these patches as 1.3 blockers no longer have auto-approval for landing. https://wiki.mozilla.org/Release_Management/B2G_Landing#Landing_Procedure_4

Comment on attachment 8376028 [details] [diff] [review] Extra fix NOTE: This flag is now for security issues only. Please see https://wiki.mozilla.org/Release_Management/B2G_Landing to better understand the B2G approval process and landings. Approval request is for both patches in this bug. [Approval Request Comment] Bug caused by (feature/regressing bug #): APZC User impact if declined: Bad rendering in Marketplace Testing completed: Not much Risk to taking this patch (and alternatives if risky): Low risk. These patches were designed to be minimal, low-risk fixes. String or UUID changes made by this patch: none

ContainerState::FindFixedPosFrameForLayerData is a bit different on b2g28 and I'm not sure how to rebase for it. Please post a branch patch.

https://hg.mozilla.org/releases/mozilla-b2g28_v1_3/rev/6e7588a3b89d https://hg.mozilla.org/releases/mozilla-b2g28_v1_3/rev/261e5eee90d7