965361 - Show firefox watermark when you hide all panels in about:home

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: Awesomescreen, defect)

Description

[Lucas Rocha (:lucasr)]

Instead of just showing a blank page.

Comment 1

[Ian Barlow (:ibarlow)]

Attachment: Screen Shot 2014-01-29 at 10.02.12 AM.png

Mockup attached. Icons can be had here: http://cl.ly/3y1H271p450y

Comment 2

[:Margaret Leibovic]

Sola, can you take this one?

Comment 3

[Sola Ogunsakin [:sola]]

sure :)

Comment 4

[Sola Ogunsakin [:sola]]

Attachment: bug-965361.patch

Comment 5

[Sola Ogunsakin [:sola]]

Attachment: Screen shot

Comment 6

[Lucas Rocha (:lucasr)]

Comment on attachment 8374587 [details] [diff] [review]
bug-965361.patch

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

Before considering something like this, have you tried simply setting a background drawable on the HomePager when it has no panels?

Comment 7

[Sola Ogunsakin [:sola]]

(In reply to Lucas Rocha (:lucasr) from comment #6)
> Comment on attachment 8374587 [details] [diff] [review]
> Before considering something like this, have you tried simply setting a
> background drawable on the HomePager when it has no panels?

Yeah I tried that, but this happened https://www.dropbox.com/s/4cz3zeli2frvdyf/Screenshot_2014-02-12-09-16-32.png

Comment 8

[Ian Barlow (:ibarlow)]

Ship it!
 
;)

Comment 9

[Ian Barlow (:ibarlow)]

Oh wait. I was jokingly referring to comment 7, and didn't see the earlier one. 

The screenshot in comment 4 actually does look great. :)

Comment 10

[Sola Ogunsakin [:sola]]

lol alright

Comment 11

[Sola Ogunsakin [:sola]]

Attachment: bug-965361.patch

Use layer drawable as background

Comment 12

[Lucas Rocha (:lucasr)]

Comment on attachment 8375042 [details] [diff] [review]
bug-965361.patch

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

You haven't mentioned whether this is working fine or not but it does look a lot simpler than your original patch, which is always a good thing :-)

::: mobile/android/base/home/HomePager.java
@@ +7,5 @@
>  
> +
> +import java.util.ArrayList;
> +import java.util.EnumSet;
> +import java.util.List;

Move this back to its original position in the file.

@@ -10,5 @@
>  import org.mozilla.gecko.animation.ViewHelper;
>  import org.mozilla.gecko.home.HomeAdapter.OnAddPanelListener;
>  import org.mozilla.gecko.home.HomeConfig.PanelConfig;
> -import org.mozilla.gecko.home.HomeConfig.PanelType;
> -import org.mozilla.gecko.util.HardwareUtils;

Please avoid including unrelated changes to your patches. If you want to do such cleanups, do so in a separate patch.

@@ +316,5 @@
> +            mTabStrip.setVisibility(View.INVISIBLE);
> +        } else {
> +            mTabStrip.setVisibility(View.VISIBLE);
> +            // Remove firefox watermark
> +            setBackgroundColor(Color.WHITE);

Instead of hard-coding the restored background here, we better just cache the original background drawable (with getDrawable()) in a class member (mOriginalBackground or something) when HomePager is first created and then set it back here. This is to avoid having to update both the layout file *and* the code if we ever change the HomePager background.

::: mobile/android/base/resources/drawable/home_pager_background.xml
@@ +1,1 @@
> +<?xml version="1.0" encoding="utf-8"?>

Rename this to home_pager_empty_state.xml for clarity?

Comment 13

[Sola Ogunsakin [:sola]]

Attachment: bug-965361.patch

Comment 14

[Lucas Rocha (:lucasr)]

Comment on attachment 8375314 [details] [diff] [review]
bug-965361.patch

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

Awesome.

Comment 15

[Lucas Rocha (:lucasr)]

https://hg.mozilla.org/integration/fx-team/rev/346678134a33

Comment 16

[Ryan VanderMeulen [:RyanVM]]

Backed out for mass Android test bustage.
https://hg.mozilla.org/integration/fx-team/rev/516606e15ef2

Comment 17

[Lucas Rocha (:lucasr)]

Pushed a fixed patch i.e. setBackgroundDrawable() instead of setBackground()
https://hg.mozilla.org/integration/fx-team/rev/360cca45a2bc

Comment 18

[Sola Ogunsakin [:sola]]

thanks Lucas :)

Comment 19

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/360cca45a2bc

Comment 20

[:Margaret Leibovic]

Sola, can you request uplift to get this on aurora?

Comment 21

[Sola Ogunsakin [:sola]]

sure

Comment 22

[Sola Ogunsakin [:sola]]

Attachment: bug-965361.patch

[Approval Request Comment]
Bug caused by (feature/regressing bug #): 965361
User impact if declined: No
Testing completed (on m-c, etc.): yes
Risk to taking this patch (and alternatives if risky): none
String or IDL/UUID changes made by this patch: no

Comment 23

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/839973474e62

Comment 24

[Teodora Vermesan (:TeoVermesan)]

The firefox watermark appears when you hide all panels in about:home, so:
Verified fixed on:
Build: Firefox for Android 29.0a2 (2014-03-05) and Firefox for Android 30.0a1 (2014-03-04)
Device: Alcatel One Touch
OS: Android 4.1.2