1137872 - Make the urlbar hiding stuff work with native pan zoom controller

Status: RESOLVED WORKSFORME

(Firefox for Android Graveyard :: Toolbar, defect)

Description

[James Willcox (:snorp) ([e-mail]) (he/him)]

There is a bunch of logic in the Java one for this currently.

Comment 1

[Chris Lord [:cwiiis]]

We do the dynamic toolbar differently in b2g - it means that bottom-fixed-pos stuff gets clipped when it's off-screen, but perhaps we should just ditch the fixed-pos margin stuff and do it similarly? If we want to keep the nice positioning of bottom-aligned fixed-pos stuff, I think it'd be best done in Gecko entirely by dynamically working out the margins based on the visible rect of the viewport frame and having some layer annotation and layers code to do the async reconciliation when the rects move.

A low-down of how we do this in b2g:

- We have an external frame and an internal browser frame. The external frame is set to scrollgrab so scrolling scrolls that first. The toolbar is in the top of this frame and the browser is also in this frame, positioned beneath the toolbar.

- Initially, this external frame is just the size of the screen. As soon as the current page grows beyond the size of the screen, the external frame is resized to be the size of the screen + the toolbar. So the DOM looks something like this:

<div id="container" scrollgrab>
  <div id="toolbar"></div>
  <iframe browser></iframe>
</div>

- The external frame never shrinks to its original size, except when navigating to a new page (this is simpler behaviour than what we have in fennec, and having dog-fooded the phone for well over a year now, I've never noticed any situations where this doesn't work out)

- Rather than trying to mess with the scroll values when the user stops scrolling so that the toolbar doesn't appear in inconsistent positions, the toolbar is fixed-position and we animate it either fully-on or fully-off depending on the scroll position of the external frame.


There are two down-sides to this vs. the fennec approach, one of which we can work out (and I intend to soon), the other of which will require some semi-significant layout/gfx work:

1- We lose any 'dead' zone, so as soon as you start scrolling upwards, the toolbar will start appearing. This is terrible for horizontal scrolling. I think we can work around this by over-sizing the external frame and using sticky-positioning on the browser frame.

2- We lose the nice async-repositioning of fixed-position content, as previously mentioned.

Comment 2

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

Paul has also been looking into this problem recently. It would be nice to use the same mechanism on all platforms if we're going to be making gecko changes to support the behavior.

https://groups.google.com/forum/m/#!topic/mozilla.dev.platform/K_NhNWVKOPI

Comment 3

[Paul Rouget [:paul]]

(In reply to Chris Lord [:cwiiis] from comment #1)
> - Rather than trying to mess with the scroll values when the user stops
> scrolling so that the toolbar doesn't appear in inconsistent positions, the
> toolbar is fixed-position and we animate it either fully-on or fully-off
> depending on the scroll position of the external frame.

What about using snap points instead? bug 945584.

Comment 4

[Chris Lord [:cwiiis]]

(In reply to Paul Rouget [:paul] from comment #3)
> (In reply to Chris Lord [:cwiiis] from comment #1)
> > - Rather than trying to mess with the scroll values when the user stops
> > scrolling so that the toolbar doesn't appear in inconsistent positions, the
> > toolbar is fixed-position and we animate it either fully-on or fully-off
> > depending on the scroll position of the external frame.
> 
> What about using snap points instead? bug 945584.

The problem here is we don't want to scroll the page when the bar is left in an intermediate state... Though perhaps we could simultaneously scroll the inner and outer frames to give the effect of the outer frame not moving (which is what we do in fennec, but it's made easier by us having direct control of the pan-zoom controller in the UI code so we can guarantee it never goes out of sync, or responds weirdly to user input) - not sure if we could really do that with snap points, it'd rely on setting new snap points on the inner frame in JS, so it's liable to get out of sync...

Also the first time I've come across scroll-snapping, looks good :) Will definitely be useful on the homescreen and task manager...

Comment 5

[Paul Rouget [:paul]]

(In reply to Chris Lord [:cwiiis] from comment #4)
> (In reply to Paul Rouget [:paul] from comment #3)
> > (In reply to Chris Lord [:cwiiis] from comment #1)
> > > - Rather than trying to mess with the scroll values when the user stops
> > > scrolling so that the toolbar doesn't appear in inconsistent positions, the
> > > toolbar is fixed-position and we animate it either fully-on or fully-off
> > > depending on the scroll position of the external frame.
> > 
> > What about using snap points instead? bug 945584.
> 
> The problem here is we don't want to scroll the page when the bar is left in
> an intermediate state...

Don't you have to if you want the position:fixed element to not be covered by the bar?

> Though perhaps we could simultaneously scroll the
> inner and outer frames to give the effect of the outer frame not moving

Tried that. It's not reliable. The 2 scroll operations might not end up in the same transaction, making the content jump.

> (which is what we do in fennec, but it's made easier by us having direct
> control of the pan-zoom controller in the UI code so we can guarantee it
> never goes out of sync, or responds weirdly to user input)

That's interesting. You can control the pan-zoom controller in Fennec? I guess it's from Java. What does this look like? Could we imagine exposing it to JS as well?

Comment 6

[Chris Lord [:cwiiis]]

(In reply to Paul Rouget [:paul] from comment #5)
> (In reply to Chris Lord [:cwiiis] from comment #4)
> > (In reply to Paul Rouget [:paul] from comment #3)
> > > (In reply to Chris Lord [:cwiiis] from comment #1)
> > > > - Rather than trying to mess with the scroll values when the user stops
> > > > scrolling so that the toolbar doesn't appear in inconsistent positions, the
> > > > toolbar is fixed-position and we animate it either fully-on or fully-off
> > > > depending on the scroll position of the external frame.
> > > 
> > > What about using snap points instead? bug 945584.
> > 
> > The problem here is we don't want to scroll the page when the bar is left in
> > an intermediate state...
> 
> Don't you have to if you want the position:fixed element to not be covered
> by the bar?

The bar is in the position:fixed element, the outer frame just has empty space above the inner frame (we should probably use the browser no-clip extension, if that still exists, but in practice, you never see this empty space).

> > Though perhaps we could simultaneously scroll the
> > inner and outer frames to give the effect of the outer frame not moving
> 
> Tried that. It's not reliable. The 2 scroll operations might not end up in
> the same transaction, making the content jump.

I wonder if we can fix this in platform(?)

> > (which is what we do in fennec, but it's made easier by us having direct
> > control of the pan-zoom controller in the UI code so we can guarantee it
> > never goes out of sync, or responds weirdly to user input)
> 
> That's interesting. You can control the pan-zoom controller in Fennec? I
> guess it's from Java. What does this look like? Could we imagine exposing it
> to JS as well?

In fennec the Java apz runs in the same process as the UI, and has various synchronisation points (maybe it even runs in the same thread? I don't recall) - I don't think we can safely expose a similar API to JS. If we did, it'd have to have some safe-guards over how long any callback could run, which of course means that it can never have truly consistent behaviour (and at that point, I'd say it's not worth doing).

Comment 7

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

Bug 1180295 largely addressed this but there might be a bit more work needed to glue stuff up with the native APZ code, so I'll leave this open for now.

Comment 8

[Dominique Vincent [:domivinc]]

After bug 1180295 (on my HTC with API 9) , I can see 2 issues (testing for another bug here [1]):

Now, the reader view tool bar is not on the bottom of the page. There is a blank area just after the tool bar. You have to scroll down to get it in the correct position:
https://youtu.be/qMD_IixH81g

And the search input field in about:config is no more accessible. As soon as you click on the search input field, the address bar is displayed. It's because the search input field should be displayed under the address bar:
https://youtu.be/U600-uGUr4k


Kats, do you think that the 2 issues will be fixed in this bug or do you want to open new bugs?

[1] https://bugzilla.mozilla.org/show_bug.cgi?id=1175536#c12

Comment 9

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

I'm in the process of fixing the issues you described in bug 1197811.

Comment 10

[Dominique Vincent [:domivinc]]

Attachment: device-2015-11-06-215611.png

Similar visual issues are visible again on Android 2.3 (see attached picture).

Comment 11

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

Looks like a new regression; I'm not sure what would have caused this. Please file a new bug and if you can, track down a regression range.

Comment 12

[Dominique Vincent [:domivinc]]

(In reply to Kartikaya Gupta (email:[e-mail]) from comment #11)
> Looks like a new regression; I'm not sure what would have caused this.
> Please file a new bug and if you can, track down a regression range.

Bug 1223144 added.

Comment 13

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

I think this is pretty much done now, we can file more specific bugs for issues that arise.