Closed Bug 962103 Opened 6 years ago Closed 6 years ago

Progress bar visual refinements

Categories

(Firefox for Android :: Theme and Visual Design, defect)

All
Android
defect
Not set

Tracking

()

RESOLVED FIXED
Firefox 31
Tracking Status
firefox29 + fixed
firefox30 + verified
firefox31 --- verified
fennec 29+ ---

People

(Reporter: ibarlow, Assigned: bnicholson)

References

(Depends on 1 open bug)

Details

Attachments

(2 files, 12 obsolete files)

13.26 KB, patch
lucasr
: review+
Details | Diff | Splinter Review
2.87 KB, patch
lucasr
: review+
Details | Diff | Splinter Review
OMG guys we have a progress bar and it's amazing! This bug is to give it a few last visual tweaks to make it look a little nicer. 

Screenshot: http://cl.ly/image/0d2t06353x2L

Changes listed in the screenshot:

* Improve visibility
** Make bar 50% thicker
** Move bar down so that only the top 1/3rd overlaps the title bar
* Make it pretty
** Add light orange to dark orange gradient to bar
** Add shadow “bulb” to right edge of bar, to look just a little more Holo-y
Note: I have some comments on the way the progress bar animates as well. That will be filed in a separate bug.
Blocks: 917896
tracking-fennec: --- → ?
I suspect that these changes will require us to move the progressbar out of the browser_toolbar.xml layout. We would add the shadow back to browser_toolbar too.
OS: Mac OS X → Android
Hardware: x86 → All
Assignee: nobody → bnicholson
tracking-fennec: ? → 29+
Status: NEW → ASSIGNED
Attached image screenshot (obsolete) —
Ian, what do you think of this screenshot?

Build here: https://dl.dropboxusercontent.com/u/35559547/fennec/progressbar-new.apk

I used the images directly from the stock browser (linked below), only I tinted them orange. They actually use a lighter color near the bulb since it creates a kind of glow effect. I tried adding a gradient effect to make the bulb darker, but they didn't look like great (I'm far from a GIMP/Photoshop pro), so I left that out that.

In case it's easier for you to just create these assets yourself and upload them, here are the Android ones I'm basing this off of:

http://androidxref.com/4.4.2_r1/xref/packages/apps/Browser/res/drawable-hdpi/progress.9.png
http://androidxref.com/4.4.2_r1/xref/packages/apps/Browser/res/drawable-mdpi/progress.9.png
http://androidxref.com/4.4.2_r1/xref/packages/apps/Browser/res/drawable-xhdpi/progress.9.png
Attachment #8371904 - Flags: feedback?(ibarlow)
Attached patch Progress bar visual refinements (obsolete) — Splinter Review
We can wait on Ian's feedback to see if the assets should be updated, but there shouldn't be any additional code changes.
Attachment #8371913 - Flags: review?(lucasr.at.mozilla)
Comment on attachment 8371913 [details] [diff] [review]
Progress bar visual refinements

Just discovered that this doesn't work well with the dynamic title bar. We'll have to figure out some way to keep them together.
Attachment #8371913 - Flags: review?(lucasr.at.mozilla)
Attached image screenshot with scrolling bug (obsolete) —
Screenshot of scrolling the page with this patch applied.
(In reply to Brian Nicholson (:bnicholson) from comment #5)
> Comment on attachment 8371913 [details] [diff] [review]
> Progress bar visual refinements
> 
> Just discovered that this doesn't work well with the dynamic title bar.
> We'll have to figure out some way to keep them together.

I think we should try a bit harder to keep the progress bar "in the toolbar" (not necessarily as a direct child of the toolbar layout). Moving it out will just cause us all sorts of headaches.

A couple of things to explore:
- Surround current toolbar with a layout that contains the current toolbar contents and and the progress bar.
- Play with margins/paddings/background drawable so that you can create the intended "effect" (progress bar partially overlapping the toolbar bottom edge) without moving it out.

The second option is the ideal one as we wouldn't be adding any extra views/layouts.
(In reply to Lucas Rocha (:lucasr) from comment #7)
> I think we should try a bit harder to keep the progress bar "in the toolbar"
> (not necessarily as a direct child of the toolbar layout). Moving it out
> will just cause us all sorts of headaches.
> 
> A couple of things to explore:
> - Surround current toolbar with a layout that contains the current toolbar
> contents and and the progress bar.
> - Play with margins/paddings/background drawable so that you can create the
> intended "effect" (progress bar partially overlapping the toolbar bottom
> edge) without moving it out.
> 
> The second option is the ideal one as we wouldn't be adding any extra
> views/layouts.

Thinking about this more, I'm starting to lean toward an approach that other browsers use: don't hide the toolbar if the page is still loading. IMO, this makes sense from a UX perspective as it keeps the current status in view, and this would solve any dynamic toolbar related issues since it remains in a static position.

If we go with that approach, it won't be critical to keep the progress bar inside of browser toolbar (even though we probably should if we can find a way, which I haven't yet).
Good point. I guess some input from Ian would be good. As well as a test build?
Attached patch 0001-dynamic-toolbar.patch (obsolete) — Splinter Review
(In reply to Mark Finkle (:mfinkle) from comment #9)
> Good point. I guess some input from Ian would be good. As well as a test
> build?

Asked Ian over IRC, but he wasn't happy with the idea; he says he likes being able to scroll the page without worrying about the current page's load state.

I can prevent the dynamic toolbar from resizing using this patch, but this includes some problems of its own. Namely, after the page has loaded and the user scrolls around, the progress bar jumps around -- though I'm sure we tweak dynamic toolbar to fix these issues.

Test build here: https://dl.dropboxusercontent.com/u/35559547/fennec/progressbar-new-dynamic-toolbar.apk
Here's a patch that keeps the progress bar inside of BrowserToolbar. Unfortunately, this has problems too: it shows orange artifacts on the page when scrolling (screenshot: https://dl.dropboxusercontent.com/u/35559547/fennec/progress-scroll-artifacts.png).

At this point, I'm convinced that we can either have an overlapping progress bar, or a progress bar that scrolls with the dynamic toolbar, but not both.
Ian, given the two options in comment 11, which would you prefer?
Flags: needinfo?(ibarlow)
Well... neither, if I'm honest. 

But if those are the only two options, I would probably go with the overlapping progress bar that keeps the title bar on screen until a page is loaded. It looks better and a little more Android like.
Flags: needinfo?(ibarlow)
(In reply to Ian Barlow (:ibarlow) from comment #13)
> Well... neither, if I'm honest. 
> 
> But if those are the only two options, I would probably go with the
> overlapping progress bar that keeps the title bar on screen until a page is
> loaded. It looks better and a little more Android like.

Sorry for the bad news -- I'll let you know if I come across a way to fix this.

Just to cover all bases: kats, would you happen to know what's causing the artifacts in the screenshot: https://dl.dropboxusercontent.com/u/35559547/fennec/progress-scroll-artifacts.png ? This is from a build using the "Redesigned progress bar inside BrowserToolbar" patch which makes the progress bar partially overlap the LayerView. This glitch happens when scrolling the dynamic toolbar up.
Flags: needinfo?(bugmail.mozilla)
Not sure unfortunately.
Flags: needinfo?(bugmail.mozilla)
Attachment #8371904 - Flags: feedback?(ibarlow)
Friendly ping -- we want to do a UX sign-off on the progress bar next Tuesday, and this is one of our last remaining blockers (if not *the* last). Any chance we can land this baby? :)
(In reply to Ian Barlow (:ibarlow) from comment #16)
> Friendly ping -- we want to do a UX sign-off on the progress bar next
> Tuesday, and this is one of our last remaining blockers (if not *the* last).
> Any chance we can land this baby? :)

Yeah, I'm finally getting around to working on this now...this will likely require some dynamic toolbar work to fix the issues in the build in comment 10.
Comment on attachment 8372522 [details] [diff] [review]
Part 1: Make progress bar overlap content

This patch from comment 11 should be good to go as-is. Alone, this patch will produce the artifacts mentioned in that comment. But if we lock the dynamic toolbar whenever progress is shown, that shouldn't be an issue.

The alternative to this patch to move the progress bar outside of BrowserToolbar, as in the patch in comment 4. Either patch will work as long as we lock the dynamic toolbar, but I went with this one since it keeps the progress bar contained.
Attachment #8372522 - Attachment description: Redesigned progress bar inside BrowserToolbar → Part 1: Make progress bar overlap content
Attachment #8372522 - Flags: review?(lucasr.at.mozilla)
Attachment #8372495 - Attachment is obsolete: true
Attachment #8371916 - Attachment is obsolete: true
Attachment #8371913 - Attachment is obsolete: true
I've been testing this patch pretty extensively today, and for whatever reason, the graphical glitches from comment 11 no longer seem to be appearing. It's possible something landed that fixed this, but I'm still skeptical and leaning more toward this just being an intermittent issue. In any case, I can't reproduce it for now, so I guess we can move forward with this patch if this is what we want. If we later find regressions -- either the glitches from comment 11 or others -- we can still use the pinned dynamic toolbar behavior I've been working on.

Here's a build with just this patch applied: http://people.mozilla.org/~bnicholson/fennec_builds/progress-bar-1.apk

Ian, what do you think of this build? This should be the behavior you wanted: a progress bar that scrolls with the dynamic toolbar. If the dynamic toolbar is scrolled of the screen, this also shows half the progress bar peeking from the top, which was something we discussed on IRC.

To anyone trying this build: please keep your eyes peeled for any orange artifacts that get left on the screen when scrolling: https://dl.dropboxusercontent.com/u/35559547/fennec/progress-scroll-artifacts.png
Flags: needinfo?(ibarlow)
Ian, here's a second build to compare: http://people.mozilla.org/~bnicholson/fennec_builds/progress-bar-2.apk

Like other browsers, this build keeps the dynamic toolbar visible whenever progress is showing.
And here are the patches used to make the build in comment 20, for reference. Not going to flag for review unless Ian prefers comment 20's build over comment 19's build.
Comment on attachment 8372522 [details] [diff] [review]
Part 1: Make progress bar overlap content

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

::: mobile/android/base/resources/layout/browser_toolbar.xml
@@ +107,5 @@
>  
>      <org.mozilla.gecko.toolbar.ToolbarProgressView android:id="@+id/progress"
>                                                     android:layout_width="fill_parent"
> +                                                   android:layout_height="16dp"
> +                                                   android:layout_marginBottom="-8dp"

Wow, 16dp? I tested the APK and it does look a bit too fat. I'd probably make slightly thinner. It's ibarlow's call though.

::: mobile/android/base/resources/layout/gecko_app.xml
@@ +17,5 @@
>           android:id="@+id/main_layout"
>           android:layout_width="fill_parent"
>           android:layout_height="fill_parent"
> +         android:background="@android:color/transparent"
> +         android:clipChildren="false">

Even in the main layout? Weird.

@@ +78,5 @@
>          <org.mozilla.gecko.widget.GeckoViewFlipper android:id="@id/browser_actionbar"
>                  android:layout_width="fill_parent"
>                  android:layout_height="@dimen/browser_toolbar_height"
>                  android:clickable="true"
> +                android:clipChildren="false"

Add comment explaining why this clipChildren is needed.
Attachment #8372522 - Flags: review?(lucasr.at.mozilla) → review+
(In reply to Brian Nicholson (:bnicholson) from comment #19)
> If we later find regressions -- either
> the glitches from comment 11 or others -- we can still use the pinned
> dynamic toolbar behavior I've been working on.
> 
> Here's a build with just this patch applied:
> http://people.mozilla.org/~bnicholson/fennec_builds/progress-bar-1.apk
> 
> Ian, what do you think of this build? This should be the behavior you
> wanted: a progress bar that scrolls with the dynamic toolbar. If the dynamic
> toolbar is scrolled of the screen, this also shows half the progress bar
> peeking from the top, which was something we discussed on IRC.


I LOVE IT

I wasn't able to break it or find any glitches either, so I would suggest we land it and get more people trying it out. 

\o/
Flags: needinfo?(ibarlow)
Well. I knew that if I left a comment as enthusiastic as that, I would invariably return with some actual feedback.

Lucas and I were talking, Brian how hard would it be to make the progress bar a little bit thinner? Like 1-2dp thinner?
After talking to ibarlow and testing different assets, landed with the following changes:
1) Progress bar is quite a bit thinner
2) Use a dark bulb instead of a glow at the tip of the bar

https://hg.mozilla.org/integration/fx-team/rev/1eebddd614eb
https://hg.mozilla.org/mozilla-central/rev/1eebddd614eb
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 30
Depends on: 977952
For reasons unknown, using clipChildren on 2.3 breaks the layout. This disables clipping on GB and earlier. The progress bar is cut off on these devices, but it still looks decent (close to what we currently have in Nightly). Screenshot at http://people.mozilla.org/~bnicholson/screenshots/fennec_progress_gb.png.
Attachment #8372522 - Attachment is obsolete: true
Attachment #8385150 - Flags: review?(lucasr.at.mozilla)
Note that testAboutHomePageNavigation, the reason this was backed out, has been disabled in bug 979038. That means we shouldn't have to worry about in failures this time around.

Also, I don't have any Honeycomb tablets, so I don't know whether Honeycomb needs this fix too. Can someone in QA verify that Honeycomb works as expected? Build here: http://people.mozilla.org/~bnicholson/fennec_builds/progress-bar-6.apk
Keywords: qawanted
Comment on attachment 8385150 [details] [diff] [review]
Make progress bar overlap content, v2

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

Ok.
Attachment #8385150 - Flags: review?(lucasr.at.mozilla) → review+
(In reply to Brian Nicholson (:bnicholson) from comment #31)
> Note that testAboutHomePageNavigation, the reason this was backed out, has
> been disabled in bug 979038. That means we shouldn't have to worry about in
> failures this time around.
> 
> Also, I don't have any Honeycomb tablets, so I don't know whether Honeycomb
> needs this fix too. Can someone in QA verify that Honeycomb works as
> expected? Build here:
> http://people.mozilla.org/~bnicholson/fennec_builds/progress-bar-6.apk
Flags: needinfo?(mihai.g.pop)
(In reply to Brian Nicholson (:bnicholson) from comment #31)
> Note that testAboutHomePageNavigation, the reason this was backed out, has
> been disabled in bug 979038. That means we shouldn't have to worry about in
> failures this time around.
> 
> Also, I don't have any Honeycomb tablets, so I don't know whether Honeycomb
> needs this fix too. Can someone in QA verify that Honeycomb works as
> expected? Build here:
> http://people.mozilla.org/~bnicholson/fennec_builds/progress-bar-6.apk

Verified this on Acer Iconia Tab A500(Android 3.2.1), with the "progress-bar-6.apk" build, and the progress bar looks and feels fine, there is no issue with the build.
Flags: needinfo?(mihai.g.pop)
https://hg.mozilla.org/mozilla-central/rev/4b5d9d01c52f
Status: REOPENED → RESOLVED
Closed: 6 years ago6 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 30
Depends on: 980438
Depends on: 980853
Depends on: 982896
Backed out in bug 981676 due to bug 981783 and bug 981676.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Keeping the progress bar inside the dynamic toolbar has resulted in several regressions, so I think it's time to give up on that approach and go back to the original plan (lock the dynamic toolbar in place during page load).
Depends on: 979166
This takes us back to the original approach of putting the toolbar in gecko_app.xml.
Attachment #8371904 - Attachment is obsolete: true
Attachment #8382824 - Attachment is obsolete: true
Attachment #8382825 - Attachment is obsolete: true
Attachment #8385150 - Attachment is obsolete: true
Attachment #8390782 - Flags: review?(lucasr.at.mozilla)
Rebased on top of fixed backout (https://bugzilla.mozilla.org/show_bug.cgi?id=981676#c8).
Attachment #8390782 - Attachment is obsolete: true
Attachment #8390782 - Flags: review?(lucasr.at.mozilla)
Attachment #8390809 - Flags: review?(lucasr.at.mozilla)
This addresses the issue from https://bugzilla.mozilla.org/attachment.cgi?id=8371916 by preventing the dynamic toolbar from moving during page load.
Attachment #8390823 - Flags: review?(lucasr.at.mozilla)
Comment on attachment 8390809 [details] [diff] [review]
Part 1: Make progress bar overlap content, v2

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

How are you going to ensure that the progress bar view is not displayed *while* the toolbar is sliding in? Try these steps:

1. Navigate to any page, scroll down so that toolbar slides off screen.
2. Tap on a link.

I haven't seen any code in these two patches that will stop the progress bar from showing before the toolbar is fully visible on screen. Am I missing something?
Attachment #8390809 - Flags: review?(lucasr.at.mozilla)
Comment on attachment 8390823 [details] [diff] [review]
Part 2: Pin the dynamic toolbar whenever progress is displayed

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

Clearing review for now to get some more background information about the expected behaviour.
Attachment #8390823 - Flags: review?(lucasr.at.mozilla)
Wondering: have you considered translating the progressbar whenever the toolbar slides so that they move around together as if the progress bar was still part of the toolbar?
(In reply to Lucas Rocha (:lucasr) from comment #44)
> Wondering: have you considered translating the progressbar whenever the
> toolbar slides so that they move around together as if the progress bar was
> still part of the toolbar?

That's a good idea. I got mcomella to test the smearing issue with this build, and he confirmed that it doesn't appear.

Build here: http://people.mozilla.org/~bnicholson/fennec_builds/progress-bar-8.apk
Attachment #8390809 - Attachment is obsolete: true
Attachment #8390823 - Attachment is obsolete: true
Attachment #8391543 - Flags: review?(lucasr.at.mozilla)
Comment on attachment 8391543 [details] [diff] [review]
Make progress bar overlap content, v3

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

Great, this is a lot simpler. I'd like to see an updated patch before giving r+. Please test this on Gingerbread devices to make sure the translation bits work fine.

::: mobile/android/base/BrowserApp.java
@@ +883,5 @@
>          ThreadUtils.postToUiThread(new Runnable() {
>              public void run() {
> +                final float translationY = marginTop - toolbarLayout.getHeight();
> +                ViewHelper.setTranslationY(toolbarLayout, translationY);
> +                mProgressView.setTranslationY(translationY);

This will crash in pre-Honeycomb. You should use ViewHelper.setTranslationY(mProgressView, translationY) instead.

::: mobile/android/base/toolbar/BrowserToolbar.java
@@ -207,5 @@
>          mMenuIcon = (GeckoImageView) findViewById(R.id.menu_icon);
>          mActionItemBar = (LinearLayout) findViewById(R.id.menu_items);
>          mHasSoftMenuButton = !HardwareUtils.hasMenuButton();
>  
> -        mProgressBar = (ToolbarProgressView) findViewById(R.id.progress);

Just do this:

mProgressBar = (ToolbarProgressView) getRootView().findViewById(R.id.progress);

And avoid the separate the progress bar setter altogether.
Attachment #8391543 - Flags: review?(lucasr.at.mozilla) → feedback+
(In reply to Lucas Rocha (:lucasr) from comment #46)
> This will crash in pre-Honeycomb. You should use
> ViewHelper.setTranslationY(mProgressView, translationY) instead.

Oops, fixed.

> ::: mobile/android/base/toolbar/BrowserToolbar.java
> @@ -207,5 @@
> >          mMenuIcon = (GeckoImageView) findViewById(R.id.menu_icon);
> >          mActionItemBar = (LinearLayout) findViewById(R.id.menu_items);
> >          mHasSoftMenuButton = !HardwareUtils.hasMenuButton();
> >  
> > -        mProgressBar = (ToolbarProgressView) findViewById(R.id.progress);
> 
> Just do this:
> 
> mProgressBar = (ToolbarProgressView)
> getRootView().findViewById(R.id.progress);
> 
> And avoid the separate the progress bar setter altogether.

I don't think this is a good idea. Having BrowserToolbar reach outside of its layout and referencing an ID it doesn't own unnecessarily increases coupling. BrowserApp will still need to hold a reference to the progress bar because of the translation, so removing the setter doesn't buy us much anyway.

Note that this is essentially the same design we use for HomeBanner, where we also have a setter: http://mxr.mozilla.org/mozilla-central/source/mobile/android/base/BrowserApp.java#1661.
Attachment #8391543 - Attachment is obsolete: true
Attachment #8392314 - Flags: review?(lucasr.at.mozilla)
Comment on attachment 8392314 [details] [diff] [review]
Make progress bar overlap content, v4

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

(In reply to Brian Nicholson (:bnicholson) from comment #47)
> > ::: mobile/android/base/toolbar/BrowserToolbar.java
> > @@ -207,5 @@
> > >          mMenuIcon = (GeckoImageView) findViewById(R.id.menu_icon);
> > >          mActionItemBar = (LinearLayout) findViewById(R.id.menu_items);
> > >          mHasSoftMenuButton = !HardwareUtils.hasMenuButton();
> > >  
> > > -        mProgressBar = (ToolbarProgressView) findViewById(R.id.progress);
> > 
> > Just do this:
> > 
> > mProgressBar = (ToolbarProgressView)
> > getRootView().findViewById(R.id.progress);
> > 
> > And avoid the separate the progress bar setter altogether.
> 
> I don't think this is a good idea. Having BrowserToolbar reach outside of
> its layout and referencing an ID it doesn't own unnecessarily increases
> coupling. BrowserApp will still need to hold a reference to the progress bar
> because of the translation, so removing the setter doesn't buy us much
> anyway.

Oh, yes, somehow missed the fact that BrowserApp needs a reference to it. But if that wasn't the case, I wouldn't worry too much about your point about coupling because the owner of the progress bar is effectively BrowserToolbar.  

> Note that this is essentially the same design we use for HomeBanner, where
> we also have a setter:
> http://mxr.mozilla.org/mozilla-central/source/mobile/android/base/BrowserApp.
> java#1661.

Yeah, and I'm not a fan of this pattern because it doesn't guarantee the caller will do the right thing.
Attachment #8392314 - Flags: review?(lucasr.at.mozilla) → review+
Inevitably, I found more issues trying this on a GB found: the progress bar never disappears once it reaches the end. Adding clearAnimation to setVisibility makes it disappear, but it reappears when setAnimation is called (triggered by AnimatorProxyPreHC). So in addition to clearing the animation when hiding, we also need to prevent future calls to setAnimation if the view isn't shown.
Attachment #8392509 - Flags: review?(lucasr.at.mozilla)
Comment on attachment 8392509 [details] [diff] [review]
Fix progress bar visibility on pre-Honeycomb devices

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

::: mobile/android/base/toolbar/ToolbarProgressView.java
@@ +102,5 @@
> +        // On GB/Froyo, setting the visibility to GONE/HIDDEN alone does not
> +        // work with translations. Calling clearAnimation acts as a workaround.
> +        if (PRE_HONEYCOMB && visibility != VISIBLE) {
> +            clearAnimation();
> +        }

I have a slight preference for clearing the animation *before* calling super.setVisibility().
Attachment #8392509 - Flags: review?(lucasr.at.mozilla) → review+
Keywords: qawanted
Target Milestone: Firefox 30 → ---
https://hg.mozilla.org/mozilla-central/rev/80a937654878
https://hg.mozilla.org/mozilla-central/rev/3d168edebd57
Status: REOPENED → RESOLVED
Closed: 6 years ago6 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 31
Comment on attachment 8392314 [details] [diff] [review]
Make progress bar overlap content, v4

[Approval Request Comment]
Bug caused by (feature/regressing bug #): New feature
User impact if declined: Progress bar is less attractive
Testing completed (on m-c, etc.): m-c
Risk to taking this patch (and alternatives if risky): Fairly low; past versions have caused regressions and were backed out, but there haven't been any reports of this breaking anything.
String or IDL/UUID changes made by this patch: None
Attachment #8392314 - Flags: approval-mozilla-beta?
Attachment #8392314 - Flags: approval-mozilla-aurora?
Attachment #8392509 - Flags: approval-mozilla-beta?
Attachment #8392509 - Flags: approval-mozilla-aurora?
Comment on attachment 8392314 [details] [diff] [review]
Make progress bar overlap content, v4

Approving because we are very early in the beta cycle. FYI, for new features, we prefer to see them land in aurora.
Attachment #8392314 - Flags: approval-mozilla-beta?
Attachment #8392314 - Flags: approval-mozilla-beta+
Attachment #8392314 - Flags: approval-mozilla-aurora?
Attachment #8392314 - Flags: approval-mozilla-aurora+
Attachment #8392509 - Flags: approval-mozilla-beta?
Attachment #8392509 - Flags: approval-mozilla-beta+
Attachment #8392509 - Flags: approval-mozilla-aurora?
Attachment #8392509 - Flags: approval-mozilla-aurora+
To the best of my knowledge this code has not been tested on varying devices, form factors and Android versions. The previous iteration of this code needed to be disabled before 29 went to beta. I am not happy that this code is on beta without those checks. Our nightly audience trends towards very new devices.
Flags: needinfo?(sledru)
Flags: needinfo?(ryanvm)
Kevin, sorry about that.
Do you think we should revert this change?
Flags: needinfo?(sledru)
Not sure what that was directed at me. I'm not the one who approved it.
Flags: needinfo?(ryanvm)
If we do it means we need to spin another beta candidate. :-( I guess we'll "just do it live". Though there is a chance of needing a backout/respin cycle.
OK. Thanks. Adding the tracking then.
Going to call this verified with https://wiki.mozilla.org/QA/Fennec/29/Beta/2/progressbar-results and my testing on aurora & nightly.
Depends on: 988527
Bah this did not make it into beta 2.
Status: VERIFIED → RESOLVED
Closed: 6 years ago6 years ago
You need to log in before you can comment on or make changes to this bug.