Build Device - Unagi Date - 11/14/2012 Steps 1. Go to marketplace in the browser 2. Scroll up and down the marketplace Expected The marketplace header should stay at the top. Actual The marketplace header experiences some evidence of chasing the top of the screen as you scroll up. This is not see while in the app, which gets me the feeling that this is a browser api bug. This is a spin off from bug 781375. The comments in there imply this has something to do with position:fixed.

My first guess is that this has something to do with the browser api, but I could be wrong. Justin - Any ideas why this issue only shows up in the browser, but works fine in an app?

> Any ideas why this issue only shows up in the browser, but works fine in an app? I have just one idea: Change TabChild::IsRootContentDocument to return true unconditionally. Does that change the behavior here?

(In reply to Justin Lebar [:jlebar] from comment #2) > > Any ideas why this issue only shows up in the browser, but works fine in an app? > > I have just one idea: Change TabChild::IsRootContentDocument to return true > unconditionally. Does that change the behavior here? Should we try putting a patch together for that and testing that on device to see what happens then? http://dxr.mozilla.org/mozilla-central/dom/ipc/TabChild.cpp.html#l1021

> Should we try putting a patch together for that and testing that on device to see what > happens then? Yes, please, if you wouldn't mind.

(In reply to Jason Smith [:jsmith] from comment #1) > My first guess is that this has something to do with the browser api, but I > could be wrong. Justin - Any ideas why this issue only shows up in the > browser, but works fine in an app? Apps / Browser use a different rendering path afaik.

> Apps / Browser use a different rendering path afaik. I believe it's specifically decided by TabChild::IsRootContentDocument. At least, that's the only difference I've noticed wrt rendering.

Maybe Chris can give a more informed hypothesis about this.

I've been looking at this with Matt Woodrow and he suggested it might be a problem with CompositorParent::TransformShadowTree() doing the wrong thing. The problem does not occur on desktop or android.

I don't think that fennec or apzc implement position:fixed in an expected way.

Comment on attachment 693234 [details] [diff] [review] Fix async scrolling of position:fixed v2 In the future, please split the addition of general helpers, refactorings, and new functionality into separate patches as much as possible. That makes reviews go faster, which means happier reviewers and faster reviews ;). >diff --git a/gfx/2d/BasePoint.h b/gfx/2d/BasePoint.h >+ Sub operator*(Sub aScale) const { I'm not sure that Point * Point = Point makes geometric sense. I think you want Point * Size = Point. But do whatever roc says ;). >diff --git a/gfx/layers/ipc/CompositorParent.cpp b/gfx/layers/ipc/CompositorParent.cpp >- shadow->SetShadowTransform(newTransform); >+ gfx3DMatrix transform(gfx3DMatrix(treeTransform) * aLayer->GetTransform()); >+ // The transform already takes the resolution scale into account. Since we >+ // will apply the resolution scale again when computing the effective >+ // transform, we must apply the inverse resolution scale here. >+ transform.Scale(1.0F/container->GetPreXScale(), >+ 1.0F/container->GetPreYScale(), Nit s/F/f/ >diff --git a/gfx/layers/ipc/ShadowLayersParent.cpp b/gfx/layers/ipc/ShadowLayersParent.cpp >- static bool fixedPositionLayersEnabled = getenv("MOZ_ENABLE_FIXED_POSITION_LAYERS") != 0; >- if (fixedPositionLayersEnabled) { >- layer->SetIsFixedPosition(common.isFixedPosition()); >- layer->SetFixedPositionAnchor(common.fixedPositionAnchor()); >- } >+ layer->SetIsFixedPosition(common.isFixedPosition()); >+ layer->SetFixedPositionAnchor(common.fixedPositionAnchor()); I see another consumer of MOZ_ENABLE_FIXED_POSITION_LAYERS in nsDisplayList. Did it not need to be changed too? >diff --git a/gfx/thebes/gfxPoint.h b/gfx/thebes/gfxPoint.h >@@ -37,14 +37,36 @@ struct THEBES_API gfxPoint : public mozilla::gfx::BasePoint<gfxFloat, gfxPoint> > // And if you need similar method which is using NS_round(), you should > // create new |RoundAwayFromZero()| method. ^^^ probably want to nuke that comment Clearing request pending answer to question above. sr to roc for gfx changes.

Comment on attachment 693234 [details] [diff] [review] Fix async scrolling of position:fixed v2 Review of attachment 693234 [details] [diff] [review]: ----------------------------------------------------------------- Yes, as Chris says, this patch should be broken up. ::: gfx/2d/BasePoint.h @@ +63,5 @@ > + return Sub(x * aScale.x, y * aScale.y); > + } > + Sub operator/(Sub aScale) const { > + return Sub(x / aScale.x, y / aScale.y); > + } Yes, these should take a size.

Try run for f5c2355ce402 is complete. Detailed breakdown of the results available here: https://tbpl.mozilla.org/?tree=Try&rev=f5c2355ce402 Results (out of 101 total builds): exception: 3 success: 73 warnings: 13 failure: 12 Builds (or logs if builds failed) available at: http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/ajones@mozilla.com-f5c2355ce402

Try run for 919786c0b3c4 is complete. Detailed breakdown of the results available here: https://tbpl.mozilla.org/?tree=Try&rev=919786c0b3c4 Results (out of 99 total builds): exception: 1 success: 85 warnings: 13 Builds (or logs if builds failed) available at: http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/ajones@mozilla.com-919786c0b3c4

https://hg.mozilla.org/integration/mozilla-inbound/rev/1310b5aab1f4 https://hg.mozilla.org/integration/mozilla-inbound/rev/955a8b88d796 https://hg.mozilla.org/integration/mozilla-inbound/rev/0e5744e4e2f6 https://hg.mozilla.org/integration/mozilla-inbound/rev/cb9dc5d37fac

https://hg.mozilla.org/mozilla-central/rev/1310b5aab1f4 https://hg.mozilla.org/mozilla-central/rev/955a8b88d796 https://hg.mozilla.org/mozilla-central/rev/0e5744e4e2f6 https://hg.mozilla.org/mozilla-central/rev/cb9dc5d37fac

Comment on attachment 693738 [details] [diff] [review] Part 4: Refactoring to use gfxPoint and gfxSize more Review of attachment 693738 [details] [diff] [review]: ----------------------------------------------------------------- Huh. Wasn't gfx::Point the new hotness?

(In reply to :Ms2ger from comment #24) > Huh. Wasn't gfx::Point the new hotness? Perhaps. I guess it depends on how much you consider Azure to be a separate thing. It also depends on how much code you want to change at any given point.

Comment on attachment 693737 [details] [diff] [review] Part 3: Added scaling operators to BaseSize and gfxPoint [Approval Request Comment] Bug caused by (feature/regressing bug #): positions:fixed has never worked on async scrolling User impact if declined: position:fixed things float across the screen when scrolling. This makes sites that use position:fixed look nasty on b2g. Testing completed: Has been in m-c since christmas. Risk to taking this patch (and alternatives if risky): May conflict with other code that sets a shadow transform. One such bug has been found and fixed. This bug also requires 825808 828249 to land on b2g18. String or UUID changes made by this patch: None.

We'll fix this web regression for v1.0.0, since our marketplace may get linked to directly.

https://hg.mozilla.org/releases/mozilla-b2g18/rev/816ed33e790f https://hg.mozilla.org/releases/mozilla-b2g18/rev/d43f324b618b https://hg.mozilla.org/releases/mozilla-b2g18/rev/e22778e3dc31 https://hg.mozilla.org/releases/mozilla-b2g18/rev/a4bfb86d0e64 These didn't apply very cleanly to b2g18, so please look the changesets over carefully to make sure no issues crept in during unbitrotting.

Landed on mozilla-b2g18 prior to the 1/25 branching to mozilla-b2g18_v1_0_0, updating status-b2g-v1.0.0

Issue doesn't reproduce on Unagi device: Build ID: 20130214070203December 5th KernelGaia: 6544fdb8dddc56f1aefe94482402488c89eeec49Gecko http://hg.mozilla.org/releases/mozilla-b2g18_v1_0_1/rev/d1288313218e marketplace header stays on top of the marketplace browser