1135814 - Content should not be under the bottom toolbar

Status: RESOLVED FIXED

(Firefox for iOS :: Browser, defect)

Description

[Stefan Arentz | :st3fan | ⏰ EST | he/him]

Content should not be under the bottom toolbar. This is going to conflict with sites that use the whole screen. Like for example maps.google.com.

This is likely the reason why only the top bar in Safari is transparent while the bottom bar is solid.

Comment 1

[Aaron Train [:aaronmt]]

Attachment: example content (screenshot)

Spotted this on Yahoo Weather, there's some text behind our toolbar a user can't read

Comment 2

[Darrin Henein [:darrin]]

I don't see the difference, we either show a solid bar (which will block content) or we let it bleed through) which will block content. Its an aesthetic thing, unless I'm misunderstanding.

This will also differ depending on device height.

Comment 3

[Stefan Arentz | :st3fan | ⏰ EST | he/him]

No the problem is that some apps go 'full screen' and kill scrolling (like google maps) so that means there is unreachable content under the bar.

The fix would be to move the webview up so that it is not under the bar at all.

Comment 4

[Darrin Henein [:darrin]]

I need to think about this...

Comment 5

[Stephen Gowan]

Attachment: Pull-Request

Hi, I was interested in working on this bug.  I just created a PR that may be useful to you.

Comment 6

[Robin Andersen [:tecgirl]]

The toolbars are designed to dock away when scrolling. Note Safari also has a bottom bar but it's fully opaque. No functionality is lost. My 2¢.

Comment 7

[Stephan Leroux [:sleroux]]

Attachment: PR https://github.com/mozilla/firefox-ios/pull/462

(Copied from Github)

Finally got this working right.

The idea now is that instead of setting contentInsets on the WKWebView's scroll view, the view is pinned to the toolbars using constraints so as they animate, the web view will expand. The additional trick in this branch is turning off the scrollView layer's maskToBounds property so the web content will render outside of the scroll view, beneath the toolbars.

Additionally, I wrapped the showToolbars call in the scrollViewWillEndDragging method with a dispatch_async block to allow the scroll view to handle it's own animation first on the main queue and run the toolbar animation after so position:fixed elements on the page don't jump around.

Last thing, I added a minimum content size for which the toolbars actually scroll away at. I noticed Safari does the same thing where if a page is too short or a fixed size, don't try to scroll away the toolbars.

Comment 8

[Wesley Johnston (:wesj)]

I would have thought the bottom contentInset would fix this and allow you to scroll "beyond" the bottom a bit. It does that on the top.

Some sites, like Google Maps also kill off all scrolling though or put all their content in an iframe inside the main frame. I assume that's the issue... sorry just thinking through this problem. The solution sounds nice to me (although I worry about page transitions and everyone always tells me that dynamically resizing a webview is a recipe for disaster).

Comment 9

[Stephan Leroux [:sleroux]]

Ya the bottom contentInset worked well for pages with position:fixed elements, but it became a problem on Google maps because the WKWebView uses it's frame as the viewport for pages it requests so Google maps returns a larger page than we wanted :(

Comment 10

[Stefan Arentz | :st3fan | ⏰ EST | he/him]

Comment on attachment 8605423 [details] [review]
PR https://github.com/mozilla/firefox-ios/pull/462

This looks great. I tested with a bunch of sites that go full screen like Google Maps and it is a huge improvement.

Comment 11

[Wesley Johnston (:wesj)]

Comment on attachment 8605423 [details] [review]
PR https://github.com/mozilla/firefox-ios/pull/462

Yeah, I'm fine too. I mostly would like some comments in the code so that the math's intent is clearer.

Comment 12

[Stephan Leroux [:sleroux]]

Hey Wes,

I updated my branch from master after you merged in your changes. Could you take a gander and make sure it's what you're expecting? I notice that the toolbars don't always animate and it's because the ScrollRecognizer is being prevented from doing the scroll logic because of this: https://github.com/mozilla/firefox-ios/blob/sleroux/Bug1135814-ContentUnderToolbar/Client/Frontend/Browser/BrowserViewController.swift#L831. I'm not too clear on what's going on with it so if you have any ideas let me know.

Comment 13

[Wesley Johnston (:wesj)]

Ahh. Looks good! Not super surprised actually. I expect the page is calling preventDefault() on a touchend event, which we're allowing to prevent our code from working (because if I didn't, the touch event listeners were locking up the page entirely).

I wonder if we can just put a show/hideToolbars call in there. i.e. is that only hit when the user is done touching...?

Comment 14

[Stephan Leroux [:sleroux]]

I did some more testing and I think I was seeing it more frequently on the mozilla.org site because theres a lot of large click areas. On normal webpages the behavior is fine. I'm going to go ahead and merge this stuff in. I did notice an issue that we can address in another bug. Since we're preventing our scroll gesture from running when the web page's gesture should be invoked for link handling, when the user scrolls after holding onto a link, the bars don't animate back since that logic doesn't get called any more.

Comment 15

[Stephan Leroux [:sleroux]]

Filed https://bugzilla.mozilla.org/show_bug.cgi?id=1166731 for link scrolling bug.

Merged PR.