Closed
Bug 950993
Opened 11 years ago
Closed 11 years ago
position:fixed CSS rule fails to keep marketplace header at top of browser
Categories
(Core :: Panning and Zooming, defect)
Tracking
()
People
(Reporter: nhirata, Assigned: cwiiis)
References
Details
(Whiteboard: [apz_testrun])
Attachments
(1 file)
1.69 KB,
patch
|
roc
:
review+
botond
:
feedback+
|
Details | Diff | Splinter Review |
Description:
Steps to Reproduce:
1. install APZ build and turn APZ on
2. launch market place
3. pan down
Expected Results: header stays at the top
Actual Results: header can move and drag along
Additional Notes:
Similar to bug 811950
Environmental Variables:
Build :
Gaia 504cf9a988cd84760b4040706ccc41e508a97fd2
Gecko 55b75ce2263e7d2a8cbfdabc7e5206d9caa89f98
BuildID 20131216140918
Version 28.0a2
ro.build.version.incremental=eng.archermind.20131114.105818
ro.build.date=Thu Nov 14 10:58:33 CST 2013
RIL Type : Mozilla RIL
OS Version: 1.3
Device: Buri
Comment 1•11 years ago
|
||
I think the uglyness is more or less expected. Milan can you confirm, that we should fix the marketplace directly and that the current behavior is the behavior for fixed header in the mobile world ? (or is there a regression somewhere) ?
Flags: needinfo?(milan)
Comment 3•11 years ago
|
||
I'm hitting bug 950540 when I try to repro so I can't see the current behaviour until that is fixed or has a workaround. Leaving needinfo flag for now.
Comment 5•11 years ago
|
||
This bug does not reproduce when the "Enable APZ" option is turned off.
Device: Buri 1.3 MOZ
BuildID: 20131216140918
Gaia: 504cf9a988cd84760b4040706ccc41e508a97fd2
Gecko: 55b75ce2263e7d2a8cbfdabc7e5206d9caa89f98
Version: 28.0a2
Firmware Version: V1.2_20131115
QA Contact: nkhristoforov
Comment 7•11 years ago
|
||
Okay - that implies this isn't a marketplace bug - it's a regression from turning APZC on across the board.
Comment 8•11 years ago
|
||
Is the marketplace app code available somewhere? From the behaviour I'm seeing it doesn't look like position:fixed in CSS, it looks like JS is moving the bar based on scroll events.
If it is done in CSS then it might be related to bug 946502.
Flags: needinfo?(bugmail.mozilla)
Comment 9•11 years ago
|
||
But to answer the question, we should be able to have a much better experience on position:fixed elements. In Fennec the behaviour is much better even though we do async panning there. We have code that is supposed to move position:fixed elements in the compositor to keep them in sync with async scrolling.
Comment 10•11 years ago
|
||
(In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #8)
> Is the marketplace app code available somewhere? From the behaviour I'm
> seeing it doesn't look like position:fixed in CSS, it looks like JS is
> moving the bar based on scroll events.
>
> If it is done in CSS then it might be related to bug 946502.
Putting needinfo on cvan to help address this question.
Flags: needinfo?(cvan)
Comment 11•11 years ago
|
||
(In reply to Jason Smith [:jsmith] from comment #10)
> (In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #8)
> > Is the marketplace app code available somewhere? From the behaviour I'm
> > seeing it doesn't look like position:fixed in CSS, it looks like JS is
> > moving the bar based on scroll events.
> >
> > If it is done in CSS then it might be related to bug 946502.
>
> Putting needinfo on cvan to help address this question.
That's incorrect - we're not using JavaScript anywhere to force the position of the header. Our header is `position: fixed`: https://github.com/mozilla/fireplace/blob/master/hearth/media/css/header.styl#L33-L35
Flags: needinfo?(cvan)
Comment 12•11 years ago
|
||
Ok, thanks for the link!
Chris, do you know what the current state of fixed-position content is with async scrolling? I recall various regressions recently but not sure if they've all been fixed or if there is anything outstanding beyond bug 946502.
Flags: needinfo?(chrislord.net)
Assignee | ||
Comment 13•11 years ago
|
||
As far as I know they were all fixed, bar background-attachment:fixed, but this is a question better answered by roc.
Flags: needinfo?(chrislord.net) → needinfo?(roc)
blocking-b2g: --- → 1.3?
Assignee | ||
Comment 15•11 years ago
|
||
It's worth noting for this that this only applies to fixed position content that *isn't* in the browser (where things behave correctly), so I would suspect that this is because in those cases, the 'root' scrollable frame is actually a sub-frame.
As far as I know, the position:fixed regressions were fixed.
Flags: needinfo?(roc)
The fixes were
https://hg.mozilla.org/mozilla-central/rev/040f7055cab9
https://hg.mozilla.org/mozilla-central/rev/576dcc232795
https://hg.mozilla.org/releases/mozilla-aurora/rev/e8523e7bddac
Then again, it's possible, even likely, that APZC for the marketplace app never worked in any Gecko build.
A MOZ_DUMP_PAINT_LIST display item/layer tree dump for the marketplace app ought to help.
Updated•11 years ago
|
Assignee: nobody → bugmail.mozilla
Comment 18•11 years ago
|
||
According to https://bugzilla.mozilla.org/show_bug.cgi?id=950488#c8 this is happening because layers aren't getting marked as fixed-position any more? Chris, do you have more details?
Flags: needinfo?(chrislord.net)
Updated•11 years ago
|
Assignee: bugmail.mozilla → bgirard
Assignee | ||
Comment 19•11 years ago
|
||
(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #17)
> The fixes were
> https://hg.mozilla.org/mozilla-central/rev/040f7055cab9
> https://hg.mozilla.org/mozilla-central/rev/576dcc232795
> https://hg.mozilla.org/releases/mozilla-aurora/rev/e8523e7bddac
> Then again, it's possible, even likely, that APZC for the marketplace app
> never worked in any Gecko build.
>
> A MOZ_DUMP_PAINT_LIST display item/layer tree dump for the marketplace app
> ought to help.
I don't have anything beyond this - I don't see fixed position content working in any non-browser APZC-enabled use-case, it would appear it's broken for all subframe scrolling (as I said in comment #15). Dumping the display-list and layer tree would help to debug this, which I could do on Monday.
I see that this is assigned to BenWa - are you working on this, or is it ok for me to take it?
Flags: needinfo?(chrislord.net) → needinfo?(bgirard)
Comment 20•11 years ago
|
||
(In reply to Chris Lord [:cwiiis] from comment #19)
> I see that this is assigned to BenWa - are you working on this, or is it ok
> for me to take it?
I though I fixed my other APZC blocker but I haven't. It would be great if you can take this so I can focus on bug 950488.
Flags: needinfo?(bgirard)
Updated•11 years ago
|
Assignee: bgirard → chrislord.net
Assignee | ||
Comment 21•11 years ago
|
||
Usefully, exporting MOZ_DUMP_PAINT_LIST causes the marketplace to crash on load, before it shows the main screen... Likely OOMing, I'll work on a reduced test-case.
Comment 22•11 years ago
|
||
(In reply to Chris Lord [:cwiiis] from comment #21)
> Usefully, exporting MOZ_DUMP_PAINT_LIST causes the marketplace to crash on
> load, before it shows the main screen... Likely OOMing, I'll work on a
> reduced test-case.
It's possible you're running into bug 946879, it happened to me when trying to do display list dumping as well (should have remembered to mention that in daily, sorry). I worked around it by commenting out some code, I can post a patch if you'd like.
Comment 23•11 years ago
|
||
(In reply to Botond Ballo [:botond] from comment #22)
> (In reply to Chris Lord [:cwiiis] from comment #21)
> > Usefully, exporting MOZ_DUMP_PAINT_LIST causes the marketplace to crash on
> > load, before it shows the main screen... Likely OOMing, I'll work on a
> > reduced test-case.
>
> It's possible you're running into bug 946879, it happened to me when trying
> to do display list dumping as well (should have remembered to mention that
> in daily, sorry). I worked around it by commenting out some code, I can post
> a patch if you'd like.
I posted my workaround patches at bug 946879.
Assignee | ||
Comment 24•11 years ago
|
||
(In reply to Botond Ballo [:botond] from comment #23)
> (In reply to Botond Ballo [:botond] from comment #22)
> > (In reply to Chris Lord [:cwiiis] from comment #21)
> > > Usefully, exporting MOZ_DUMP_PAINT_LIST causes the marketplace to crash on
> > > load, before it shows the main screen... Likely OOMing, I'll work on a
> > > reduced test-case.
> >
> > It's possible you're running into bug 946879, it happened to me when trying
> > to do display list dumping as well (should have remembered to mention that
> > in daily, sorry). I worked around it by commenting out some code, I can post
> > a patch if you'd like.
>
> I posted my workaround patches at bug 946879.
This is handy, but I'm just dumping the paint list, not painting itself.
I tried to produce a simple test-case, but it seems position:fixed works fine in my test case... So now I need to have a look at what the marketplace is doing exactly.
Assignee | ||
Comment 25•11 years ago
|
||
Oh, ok, the marketplace is just quitting on load, regardless of MOZ_DUMP_PAINT_LIST... I guess I'm also using a debug build now vs. earlier, perhaps that's affecting things... Not useful.
Comment 26•11 years ago
|
||
(In reply to Chris Lord [:cwiiis] from comment #24)
> (In reply to Botond Ballo [:botond] from comment #23)
> > (In reply to Botond Ballo [:botond] from comment #22)
> > > (In reply to Chris Lord [:cwiiis] from comment #21)
> > > > Usefully, exporting MOZ_DUMP_PAINT_LIST causes the marketplace to crash on
> > > > load, before it shows the main screen... Likely OOMing, I'll work on a
> > > > reduced test-case.
> > >
> > > It's possible you're running into bug 946879, it happened to me when trying
> > > to do display list dumping as well (should have remembered to mention that
> > > in daily, sorry). I worked around it by commenting out some code, I can post
> > > a patch if you'd like.
> >
> > I posted my workaround patches at bug 946879.
>
> This is handy, but I'm just dumping the paint list, not painting itself.
Ah, I didn't realize there was a difference! I also just wanted to dump the paint list, so a simpler workaround in my case would have been to define MOZ_DUMP_PAINT_LIST instead of MOZ_DUMP_PAINT...
(In reply to Chris Lord [:cwiiis] from comment #25)
> Oh, ok, the marketplace is just quitting on load, regardless of
> MOZ_DUMP_PAINT_LIST... I guess I'm also using a debug build now vs. earlier,
> perhaps that's affecting things... Not useful.
Another bug I've run into when running a debug build of B2G is bug 820716. My workaround for that is to comment out http://mxr.mozilla.org/mozilla-central/source/toolkit/crashreporter/nsExceptionHandler.cpp#2496.
Assignee | ||
Comment 27•11 years ago
|
||
ok, so:
Works:
- position:fixed div in browser
- position:fixed div in wrapper
- position:fixed div as installed app
- Firefox Marketplace in browser
Doesn't work:
- Firefox marketplace as installed app
The layer tree for the marketplace, I think:
ClientLayerManager (0x404250d0)
ClientContainerLayer (0x4383b000) [visible=< (x=0, y=0, w=540, h=930); >] [opaqueContent] [metrics={ viewport=(x=0.000000, y=0.000000, w=360.000000, h=620.000000) viewportScroll=(x=0.000000, y=0.000000) displayport=(x=0.000000, y=0.000000, w=360.000000, h=620.000000) scrollId=2 }]
ClientThebesLayer (0x432f90c0) [not visible] [opaqueContent]
ClientContainerLayer (0x467f4c00) [clip=(x=0, y=0, w=540, h=930)] [visible=< (x=0, y=0, w=540, h=930); >] [opaqueContent]
ClientContainerLayer (0x467f5000) [clip=(x=0, y=0, w=540, h=930)] [visible=< (x=0, y=0, w=540, h=3255); >] [opaqueContent] [metrics={ viewport=(x=0.000000, y=0.000000, w=360.000000, h=620.000000) viewportScroll=(x=0.000000, y=0.000000) displayport=(x=0.000000, y=0.000000, w=360.000000, h=2170.000000) scrollId=7 }]
ClientThebesLayer (0x432fbaf0) [visible=< (x=0, y=137, w=540, h=3118); >] [opaqueContent] [valid=< (x=0, y=137, w=540, h=3118); >]
ClientThebesLayer (0x432fbca0) [transform=[ 1 0; 0 1; 0 72; ]] [visible=< (x=0, y=0, w=540, h=3); >] [isFixedPosition anchor=0.000000,-72.000000 margin=0.000000,0.000000,0.000000,-1.000000] [valid=< (x=0, y=0, w=540, h=32); >]
ClientThebesLayer (0x432fbe50) [visible=< (x=0, y=72, w=540, h=65); >] [opaqueContent] [valid=< (x=0, y=72, w=540, h=65); >]
ClientThebesLayer (0x44ce1180) [visible=< (x=0, y=0, w=540, h=72); >] [opaqueContent] [isFixedPosition anchor=0.000000,0.000000 margin=0.000000,0.000000,0.000000,-1.000000] [valid=< (x=0, y=0, w=540, h=72); >]
Assuming that the fixed-pos header is the last layer in that list, it is marked as fixed position... (the visible region seems about right) So the new question, I suppose, is why it isn't being transformed when it's an app. Will delve further tomorrow.
Assignee | ||
Comment 28•11 years ago
|
||
An interesting thing to note in this layer tree, is that it has two viewports, and the fixed content is a child of the second one.
Assignee | ||
Comment 29•11 years ago
|
||
Having slept on it, comment #28 is almost certainly why this doesn't work - I'm not sure why this app ends up with two viewports, but the scroll offset likely needs to be accumulated when a second viewport is hit like this.
The two possible fixes here are to change the marketplace so it doesn't trigger this behaviour, or to fix the async scroll transform code so that it handles it. I'm going to attempt the latter.
Assignee | ||
Comment 30•11 years ago
|
||
So the problem here is that the root frame has a viewport, and if it does, ApplyAsyncContentTransformToTree gets called and the 'fallback' path, that would do this recursively, doesn't run.
I think, now that we have browser iframes and such, that the fallback path needs to always run, but that AlignFixedAndStickyLayers needs to also stop when it encounters scrollable layers. Something along these lines anyway. I'll write the patch and see if the results make sense.
Assignee | ||
Comment 31•11 years ago
|
||
So, this fixes the issue (and is much simpler than what I was waffling about in the last comment). I'm not sure I have this right, but it seems right to me.
Note, that sometimes the header flickers and disappears when scrolling in the marketplace, I'm not sure if this is because of some weirdness on my Peak (I'm seeing some odd behaviour elsewhere on it that I wouldn't expect), because the untransformed fixed layer ends up falling outside of the displayport, or some other reason. It's likely we'll want to fix this too, and I have a feeling this may need a layout-side fix.
Attachment #8359812 -
Flags: review?(roc)
Attachment #8359812 -
Flags: feedback?(botond)
Comment 32•11 years ago
|
||
Comment on attachment 8359812 [details] [diff] [review]
Fix async scrolling of fixed layers in scrollable sub-layers
Review of attachment 8359812 [details] [diff] [review]:
-----------------------------------------------------------------
Looks reasonable to me. (I'm not extensively familiar with this code though.)
Attachment #8359812 -
Flags: feedback?(botond) → feedback+
Comment on attachment 8359812 [details] [diff] [review]
Fix async scrolling of fixed layers in scrollable sub-layers
Review of attachment 8359812 [details] [diff] [review]:
-----------------------------------------------------------------
Looks good, thanks!
Attachment #8359812 -
Flags: review?(roc) → review+
Assignee | ||
Comment 35•11 years ago
|
||
Pushed to inbound: https://hg.mozilla.org/integration/mozilla-inbound/rev/fda811732305
Status: NEW → ASSIGNED
Updated•11 years ago
|
status-b2g-v1.2:
--- → wontfix
status-b2g-v1.3:
--- → affected
status-b2g-v1.4:
--- → affected
status-firefox27:
--- → wontfix
status-firefox28:
--- → affected
status-firefox29:
--- → affected
Comment 36•11 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla29
Updated•11 years ago
|
Comment 37•11 years ago
|
||
You need to log in
before you can comment on or make changes to this bug.
Description
•