Adjust bottom fixed position to visible viewport while toolbar is transitioning
Categories
(GeckoView :: Toolbar, defect)
Tracking
(firefox66 wontfix, firefox67 wontfix, firefox68 fixed)
People
(Reporter: eeejay, Assigned: eeejay)
References
(Blocks 1 open bug)
Details
(Whiteboard: [geckoview:fenix:m4])
Attachments
(2 files)
Assignee | ||
Comment 1•6 years ago
|
||
Updated•6 years ago
|
Comment 2•6 years ago
|
||
Eitan what's next for this bug, do you want to request review?
Assignee | ||
Comment 3•6 years ago
|
||
I'm going to unassign myself for now so other folks can jump in on it. I may have time later to look into it again.
Assignee | ||
Comment 4•6 years ago
|
||
Some further deets about this work. Depending on time, I might pick it up again. But I'm first attending to my team's priorities.
- I think the approach I took in this bug is good for the transitional state when the toolbar is half visible. Besides this, I think we need to have a good implementation of an initial container block (ICB) vs. viewport (bug 1515980). Essentially, there would an initial containing block (ICB) that should be the viewport size minus the toolbar height. CSS
height: 100%
would match this. Then there would be the dynamic viewport height that grows when the toolbar goes away. CSSheight: 100vh
would match this. - We need to disable the toolbar from hiding when dealing with fullscreen content (ie.
body
height of less than 100%). - We need to design a public GV API that does all those three things (transitional state, ICB vs viewport, and tell a-c that content is not scrollable). This wip has something that answers the first two, that may be good enough for now.
This could probably be tackled piecemeal, with this bug getting resolved first. But since I am not very familiar with layout code I don't know whats best.
Here are some explainers on ICV vs viewport:
Thanks for investigating Eitan. I spoke to David about prioritizing this bug as this issue is a potential release blocker for Fenix.
Chris, sounds like fixing this bug is the first step, and the rest of the work also need to be filed and tracked and prioritized.
Sebastian, I am copying you here in case Eitan has questions about the layout code.
Comment 7•6 years ago
|
||
This problem also appears to affect https://www.digg.com/ The bottom navigation bar appears to be hidden.
Comment 9•6 years ago
|
||
Other problem sites:
- Reddit bottom nav bar
- Quora bottom nav bar
- flights.google.com: the airport search mode's Cancel/Apply buttons are hidden.
Updated•6 years ago
|
Comment 10•6 years ago
|
||
Removing my NI. Let me know if this ends up needing GV engineering.
Comment 11•6 years ago
|
||
@hiro: Can you take some time to investigate this perhaps this week or next and give some details on what work is required from our side – and approximate scope? Per comment 5, it's a potential blocker for Fenix. Looks like it might be dependent on bug 1515980.
Comment 12•6 years ago
|
||
IIUC what the problem in this bug is, this bug doesn't need to depend on bug 1515980.
It seems to me that bug 1515980 is for providing a way to notify the both of sizes of viewport-ish (the one is the size when the urlbar is hidden, the other is the size when the urlbar is shown, we might just need the urlbar size though) and Gecko uses the size for layout.
(In reply to Eitan Isaacson [:eeejay] from comment #4)
- I think the approach I took in this bug is good for the transitional state when the toolbar is half visible. Besides this, I think we need to have a good implementation of an initial container block (ICB) vs. viewport (bug 1515980). Essentially, there would an initial containing block (ICB) that should be the viewport size minus the toolbar height. CSS
height: 100%
would match this. Then there would be the dynamic viewport height that grows when the toolbar goes away. CSSheight: 100vh
would match this.
Correction: 100vh is not changed at all, it's always the size without the utlbar. i.e the size as if the urlbar is hidden.
As for this bug, we need a way to notify the transitioning height of the urlbar to Gecko which is exactly what Eitan did in attachment 9033045 [details]. (I think the API should also take top offset value because I believe toolbar's position dependns on applications). Anyways, it's more related to APZ stuff, not layout.
What the most challending task here to me is that how we can synchronize APZ and the urlbar transition (I assume the urlbar is rendered by application).
CCing kats, I believe he is the best person to ask (And I guess he has already some ideas since bug 1498699).
Comment 13•6 years ago
|
||
(In reply to Hiroyuki Ikezoe (:hiro) from comment #12)
CCing kats, I believe he is the best person to ask (And I guess he has already some ideas since bug 1498699).
I don't think bug 1498699 is relevant here. This bug basically sounds like yet another dynamic toolbar implementation. Randall wrote the last one and can probably answer stuff here too. We can probably reuse at least some of the code/plumbing we added for his implementation.
In particular I think you'll want to call AsyncCompositionManager::SetFixedLayerMargins with the bottom value set to the visible height of the toolbar. That will cause things like position-fixed items and scrollbars to get scrunched upwards to not overlap/go under the toolbar.
It's not really clear to me from the discussion here exactly what the desired behaviour is but if that can be clarified I can try to help more. I suppose I should also install Fenix and see what the behaviour looks like now...
Comment 14•6 years ago
|
||
(In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #13)
(In reply to Hiroyuki Ikezoe (:hiro) from comment #12)
CCing kats, I believe he is the best person to ask (And I guess he has already some ideas since bug 1498699).
I don't think bug 1498699 is relevant here. This bug basically sounds like yet another dynamic toolbar implementation.
That's what I meant because you left a comment that 'GeckoView-based products won't be using it (at least for now)' in that bug. I think now GeckoView wants their own dynamic toolbar machinery.
Comment 15•6 years ago
|
||
Resetting [geckoview]
whiteboard tag so GeckoView triage will review this bug's recent discussion about dynamic toolbars.
Updated•6 years ago
|
Comment 16•6 years ago
|
||
(In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #13)
It's not really clear to me from the discussion here exactly what the desired behaviour is but if that can be clarified I can try to help more. I suppose I should also install Fenix and see what the behaviour looks like now...
I have asked Fenix Product and UX people for input on the desired behavior.
Comment 17•6 years ago
|
||
Based on Hiro's input from comment 12 and comment 13, sounds like we don't need further layout team discussion for now.
Assignee | ||
Updated•6 years ago
|
Updated•6 years ago
|
Assignee | ||
Updated•6 years ago
|
Updated•6 years ago
|
Updated•6 years ago
|
Comment 19•6 years ago
|
||
The corresponding AC issue is mozilla-mobile/android-components#2675.
Updated•6 years ago
|
Comment 20•6 years ago
|
||
Comment 21•6 years ago
|
||
bugherder |
Comment 22•5 years ago
|
||
While testing out the changes :botond had implemented for the dynamic toolbar, I noticed that this issue isn't really fixed in my testing on a lot of sites.
Example sites in the screenshots attached:
- https://bokand.github.io/demo/urlbarsize.html (Chrome test site)
- https://reddit.com (app or web banner)
- https://theguardian.com (privacy banner)
- https://flights.google.com (airport searching cuts out search bar on top)
Re-opening to put this back on the GV board.
:cpeterson, we're not landing the dynamic toolbar in Fenix yet since these are quire common sites that we would get reports on.
Updated•5 years ago
|
Comment 23•5 years ago
|
||
:cpeterson, we're not landing the dynamic toolbar in Fenix yet since these are quire common sites that we would get reports on.
I will send this bug back to GV triage. We might want to file a new bug for these specific site issues.
Comment 24•5 years ago
|
||
In comment 4, the same demo site was used for testing but it doesn't seem to be working on it, so I'm not sure if this would be regarded as a new issue.
Comment 25•5 years ago
|
||
(In reply to Jonathan Almeida [:jonalmeida] from comment #22)
Example sites in the screenshots attached:
- https://bokand.github.io/demo/urlbarsize.html (Chrome test site)
This one hasn't yet been fixed. Bug 1586144 will fix some of problems there, at least the initial rendering result should be the same as Chrome once it landed (we also need to change android-coments though).
Comment 26•5 years ago
|
||
(In reply to Jonathan Almeida [:jonalmeida] from comment #22)
- https://theguardian.com (privacy banner)
The banner looks position:sticky, it's worth filing a new bug for this case.
Comment 27•5 years ago
|
||
(In reply to Hiroyuki Ikezoe (:hiro) from comment #26)
The banner looks position:sticky, it's worth filing a new bug for this case.
I've filed https://bugzilla.mozilla.org/show_bug.cgi?id=1594451 for this one. Does that mean we should close this? Is is the same case for all the other sites too?
Comment 28•5 years ago
|
||
(In reply to Jonathan Almeida [:jonalmeida] from comment #27)
(In reply to Hiroyuki Ikezoe (:hiro) from comment #26)
The banner looks position:sticky, it's worth filing a new bug for this case.
I've filed https://bugzilla.mozilla.org/show_bug.cgi?id=1594451 for this one. Does that mean we should close this? Is is the same case for all the other sites too?
Thanks for filing that bug. Yes, I think we can close this bug. I don't see any issues on reddit.com and flights.google.com with the fix for bug 1586144. Though I haven't checked the sites with a bare reference-browser, it hopefully will be fixed by the same bug.
Comment 29•5 years ago
|
||
Thanks for filing that bug. Yes, I think we can close this bug. I don't see any issues on reddit.com and flights.google.com with the fix for bug 1586144.
Thanks. I'll close this bug again and we can track the new work in bug 1586144.
Comment 30•5 years ago
|
||
(In reply to Jonathan Almeida [:jonalmeida] from comment #22)
Created attachment 9106198 [details]
broken sites after fix.pngWhile testing out the changes :botond had implemented for the dynamic toolbar, I noticed that this issue isn't really fixed in my testing on a lot of sites.
Example sites in the screenshots attached:
- https://bokand.github.io/demo/urlbarsize.html (Chrome test site)
What is the actual issue on this test page? It's not clear to me.
Comment 31•5 years ago
|
||
(In reply to Botond Ballo [:botond] from comment #30)
(In reply to Jonathan Almeida [:jonalmeida] from comment #22)
Created attachment 9106198 [details]
broken sites after fix.pngWhile testing out the changes :botond had implemented for the dynamic toolbar, I noticed that this issue isn't really fixed in my testing on a lot of sites.
Example sites in the screenshots attached:
- https://bokand.github.io/demo/urlbarsize.html (Chrome test site)
What is the actual issue on this test page? It's not clear to me.
Only the bottom of the 2nd and 4th bars should both be seen.
Comment 32•2 years ago
|
||
Moving toolbar bugs to the new GeckoView::Toolbar component.
Description
•