The progress bar is barely visible in full screen browsing

VERIFIED FIXED in Firefox 40

Status

()

Firefox for Android
General
VERIFIED FIXED
3 years ago
3 years ago

People

(Reporter: csuciu, Assigned: mhaigh)

Tracking

39 Branch
Firefox 40
ARM
Android
Points:
---

Firefox Tracking Flags

(firefox36 affected, firefox37 affected, firefox38 affected, firefox39 affected, firefox40 verified)

Details

Attachments

(4 attachments, 3 obsolete attachments)

(Reporter)

Description

3 years ago
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
(Reporter)

Comment 1

3 years ago
Created attachment 8575949 [details]
photo_2015-03-11_14-11-16.jpg

For comparison, this is how it looks when the url bar is in view
ni Darrin for UX input on this bug.
Flags: needinfo?(dhenein)
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?
Flags: needinfo?(dhenein) → needinfo?(alam)
(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 (?).
Flags: needinfo?(alam) → needinfo?(catalin.suciu)
(Reporter)

Comment 5

3 years ago
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)
Flags: needinfo?(catalin.suciu)
(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.
Flags: needinfo?(mhaigh)
(Reporter)

Comment 7

3 years ago
>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.
(Assignee)

Updated

3 years ago
Assignee: nobody → mhaigh
Flags: needinfo?(mhaigh)
(Assignee)

Comment 8

3 years ago
Created attachment 8576734 [details] [diff] [review]
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.
Attachment #8576734 - Flags: review?(margaret.leibovic)
Brian - Do you remember any gotcha's from the initial implementation? You played around with changing the size IIRC.
Flags: needinfo?(bnicholson)

Comment 10

3 years ago
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?
Attachment #8576734 - Flags: review?(margaret.leibovic)
Attachment #8576734 - Flags: review?(bnicholson)
Attachment #8576734 - Flags: review+
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?
Attachment #8576734 - Flags: review?(bnicholson) → review+
Flags: needinfo?(bnicholson)
(Assignee)

Comment 12

3 years ago
Created attachment 8580103 [details]
Screenshot of updated progress bar position - fullscreen
(Assignee)

Comment 13

3 years ago
Created attachment 8580105 [details]
Screenshot of updated progress bar position - not fullscreen
(Assignee)

Comment 14

3 years ago
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?
Flags: needinfo?(alam)
(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.
Flags: needinfo?(alam) → needinfo?(mhaigh)
(Assignee)

Comment 16

3 years ago
Created attachment 8590230 [details] [diff] [review]
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...
Attachment #8576734 - Attachment is obsolete: true
Flags: needinfo?(mhaigh)
Attachment #8590230 - Flags: review?(margaret.leibovic)
(Assignee)

Comment 17

3 years ago
Created attachment 8590231 [details]
Chrome completely scrolled offscreen
Attachment #8580103 - Attachment is obsolete: true
Attachment #8580105 - Attachment is obsolete: true

Comment 18

3 years ago
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 :)
Attachment #8590230 - Flags: review?(margaret.leibovic) → review+
https://hg.mozilla.org/mozilla-central/rev/820e9eab50bc
Status: NEW → RESOLVED
Last Resolved: 3 years ago
status-firefox40: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 40
(Reporter)

Comment 22

3 years ago
Tested on several devices, in portrait and landscape, and didn't see anything wrong.
Verifying as fixed on Nightly 40.0a1 (2015-04-19)
Status: RESOLVED → VERIFIED
status-firefox40: fixed → verified
You need to log in before you can comment on or make changes to this bug.