1142012 - The progress bar is barely visible in full screen browsing

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Catalin Suciu [:csuciu]]

Attachment: photo_2015-03-11_14-11-19.jpg

Steps: 
1. Start loading any page   
2. While the page is still loading, hide the url bar and check the progress bar

Result: The progress bar is barely visible 

Please see the screenshot

Comment 1

[Catalin Suciu [:csuciu]]

Attachment: photo_2015-03-11_14-11-16.jpg

For comparison, this is how it looks when the url bar is in view

Comment 2

[Lawrence Mandel [:lmandel] (use needinfo)]

ni Darrin for UX input on this bug.

Comment 3

[Darrin Henein [:darrin]]

This seems like something we should fix, by nudging the progress bar down a few pixels so it properly aligns with the top of the screen.

Thoughts Anthony?

Comment 4

[Anthony Lam (:antlam) Find me, @antlam on Slack!]

(In reply to Catalin Suciu from comment #0)
> Created attachment 8575944 [details]
> photo_2015-03-11_14-11-19.jpg
> 
> Steps: 
> 1. Start loading any page   
> 2. While the page is still loading, hide the url bar and check the progress
> bar
> 
> Result: The progress bar is barely visible 
> 
> Please see the screenshot

How're you hiding the URL bar /Toolbar in this example? What device is this?

If this is a Full Screen issue, then we should probably move it down 2 dp (?).

Comment 5

[Catalin Suciu [:csuciu]]

I'm hiding the url bar by panning up the page while the content is still loading . The screenshots are from Nexus 5 (5.0.2)

Comment 6

[Anthony Lam (:antlam) Find me, @antlam on Slack!]

(In reply to Catalin Suciu from comment #5)
> I'm hiding the url bar by panning up the page while the content is still
> loading . The screenshots are from Nexus 5 (5.0.2)

Thanks Catalin!

Martyn, do you have some time to take a look at this? the simple solution would be to pad it down 2 dp (or 1 dp) and then see how it goes. I'm also wondering about Tablet but I don't have one on me ATM.

Comment 7

[Catalin Suciu [:csuciu]]

>I'm also wondering about Tablet but I don't have one on me ATM.

Tablets have the same issue. I've checked on Nexus 7.

Comment 8

[Martyn Haigh (:mhaigh)]

Attachment: The progress bar is barely visible in full screen browsing

A 1dp change seems to be all that's needed on my current test devices.

Comment 9

[Mark Finkle (:mfinkle) (use needinfo?)]

Brian - Do you remember any gotcha's from the initial implementation? You played around with changing the size IIRC.

Comment 10

[:Margaret Leibovic]

Comment on attachment 8576734 [details] [diff] [review]
The progress bar is barely visible in full screen browsing

Review of attachment 8576734 [details] [diff] [review]:
-----------------------------------------------------------------

::: mobile/android/base/resources/layout/gecko_app.xml
@@ +126,4 @@
>          <org.mozilla.gecko.toolbar.ToolbarProgressView android:id="@+id/progress"
>                                                         android:layout_width="match_parent"
>                                                         android:layout_height="14dp"
> +                                                       android:layout_marginTop="-7dp"

This looks like a very reasonable change to me, but mfinkle's last comment has me worried that bnicholson should take a look before we land this :)

Does this change how the progress bar looks when the toolbar is still visible?

Comment 11

[Brian Nicholson (:bnicholson)]

Comment on attachment 8576734 [details] [diff] [review]
The progress bar is barely visible in full screen browsing

Review of attachment 8576734 [details] [diff] [review]:
-----------------------------------------------------------------

(In reply to Mark Finkle (:mfinkle) from comment #9)
> Brian - Do you remember any gotcha's from the initial implementation? You
> played around with changing the size IIRC.

Yeah, there were plenty of crazy things happening like graphics crashes and artifacts appearing over the surface view. They were all related to layered drawables with transparency, though, so a small tweak to the margin doesn't concern me much.

It will change the progress bar positioning when the toolbar is visible as Margaret said, so that's something to look out for. In particular, the toolbar has a small shadow on the bottom, and the progress bar overlaps that shadow exactly (see second attached screenshot). Does it look off balance with this change?

Comment 12

[Martyn Haigh (:mhaigh)]

Attachment: Screenshot of updated progress bar position - fullscreen

Comment 13

[Martyn Haigh (:mhaigh)]

Attachment: Screenshot of updated progress bar position - not fullscreen

Comment 14

[Martyn Haigh (:mhaigh)]

I think it looks alright - see screenshots above.  The bar is now visible when fullscreen but it is not quite sitting on the bottom of the UI when not fullscreen.

What do you think Anthony?

Comment 15

[Anthony Lam (:antlam) Find me, @antlam on Slack!]

(In reply to Martyn Haigh (:mhaigh) from comment #14)
> I think it looks alright - see screenshots above.  The bar is now visible
> when fullscreen but it is not quite sitting on the bottom of the UI when not
> fullscreen.
>
> What do you think Anthony?

So, it looks better in attachment 8580103 [details] but it definitely needs to sit back in the "track" when the toolbar UI is visible. We'll need to address this before resolving this bug I think.

Comment 16

[Martyn Haigh (:mhaigh)]

Attachment: The progress bar is barely visible in full screen browsing

Different approach - here we cap the amount the progress bar can shift offscreen.  This doesn't affect the position of the progress bar except when the chrome is pretty much completely gone, at which point the progress bar will stop moving.

I've had to hardcode a value here as we can't programatically generate the 'correct' amount to shift the view down by.  This is because the progress view is a lot larger than the visible part of it and the image we use for the progress is a lot taller than the part you can see (progress.9.png).

Screenshot coming up...

Comment 17

[Martyn Haigh (:mhaigh)]

Attachment: Chrome completely scrolled offscreen

Comment 18

[:Margaret Leibovic]

Comment on attachment 8590230 [details] [diff] [review]
The progress bar is barely visible in full  screen browsing

Review of attachment 8590230 [details] [diff] [review]:
-----------------------------------------------------------------

r+ with comments addressed.

::: mobile/android/base/BrowserApp.java
@@ +1422,5 @@
>                  ViewHelper.setTranslationY(browserChrome, translationY);
> +
> +                // Stop the progressView from moving all the way up so that we can still see a good chunk of it
> +                // when the chrome is offscreen.
> +                final float progressTranslationY = Math.max(marginTop - browserChrome.getHeight(), 4 - browserChrome.getHeight());

Is it okay to use "4" directly here, or should we be using getDimensionPixelSize for a resource value?

Also, I want us to make sure this doesn't have any impact on page layout, since I know that the dynamic toolbar has historically caused a bunch of issues with that. I don't *think* this should cause issues, if we're just making the progress view overlap the content, but you never know :)

Comment 19

[Martyn Haigh (:mhaigh)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=2b9ad61dd805

Comment 20

[Martyn Haigh (:mhaigh)]

https://hg.mozilla.org/integration/fx-team/rev/820e9eab50bc

Comment 21

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/820e9eab50bc

Comment 22

[Catalin Suciu [:csuciu]]

Tested on several devices, in portrait and landscape, and didn't see anything wrong.
Verifying as fixed on Nightly 40.0a1 (2015-04-19)