Closed Bug 965361 Opened 6 years ago Closed 6 years ago

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

Categories

(Firefox for Android :: Awesomescreen, defect)

All
Android
defect
Not set

Tracking

()

RESOLVED FIXED
Firefox 30
Tracking Status
firefox29 --- verified
firefox30 --- verified
fennec 29+ ---

People

(Reporter: lucasr, Assigned: oogunsakin)

References

(Blocks 1 open bug)

Details

(Whiteboard: shovel-ready)

Attachments

(3 files, 3 obsolete files)

Instead of just showing a blank page.
Mockup attached. Icons can be had here: http://cl.ly/3y1H271p450y
Whiteboard: shovel-ready
tracking-fennec: --- → ?
Sola, can you take this one?
Assignee: nobody → oogunsakin
tracking-fennec: ? → 29+
sure :)
Attached patch bug-965361.patch (obsolete) — Splinter Review
Attachment #8374587 - Flags: review?(lucasr.at.mozilla)
Attached image Screen shot
Flags: needinfo?(ibarlow)
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?
Attachment #8374587 - Flags: review?(lucasr.at.mozilla)
(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
Ship it!
 
;)
Flags: needinfo?(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. :)
lol alright
Attached patch bug-965361.patch (obsolete) — Splinter Review
Use layer drawable as background
Attachment #8374587 - Attachment is obsolete: true
Attachment #8375042 - Flags: feedback?(lucasr.at.mozilla)
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?
Attachment #8375042 - Flags: feedback?(lucasr.at.mozilla) → feedback+
Attached patch bug-965361.patch (obsolete) — Splinter Review
Attachment #8375042 - Attachment is obsolete: true
Attachment #8375314 - Flags: review?(lucasr.at.mozilla)
Comment on attachment 8375314 [details] [diff] [review]
bug-965361.patch

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

Awesome.
Attachment #8375314 - Flags: review?(lucasr.at.mozilla) → review+
Keywords: checkin-needed
Pushed a fixed patch i.e. setBackgroundDrawable() instead of setBackground()
https://hg.mozilla.org/integration/fx-team/rev/360cca45a2bc
thanks Lucas :)
https://hg.mozilla.org/mozilla-central/rev/360cca45a2bc
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 30
Sola, can you request uplift to get this on aurora?
Flags: needinfo?(oogunsakin)
sure
Attached patch bug-965361.patchSplinter Review
[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
Attachment #8384726 - Flags: approval-mozilla-aurora?
Flags: needinfo?(oogunsakin)
Attachment #8384726 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Attachment #8375314 - Attachment is obsolete: true
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
You need to log in before you can comment on or make changes to this bug.