Closed
Bug 965361
Opened 8 years ago
Closed 8 years ago
Show firefox watermark when you hide all panels in about:home
Categories
(Firefox for Android Graveyard :: Awesomescreen, defect)
Tracking
(firefox29 verified, firefox30 verified, fennec29+)
RESOLVED
FIXED
Firefox 30
People
(Reporter: lucasr, Assigned: oogunsakin)
References
Details
(Whiteboard: shovel-ready)
Attachments
(3 files, 3 obsolete files)
61.26 KB,
image/png
|
Details | |
40.42 KB,
image/png
|
Details | |
22.39 KB,
patch
|
Sylvestre
:
approval-mozilla-aurora+
|
Details | Diff | Splinter Review |
Instead of just showing a blank page.
Comment 1•8 years ago
|
||
Mockup attached. Icons can be had here: http://cl.ly/3y1H271p450y
Updated•8 years ago
|
Whiteboard: shovel-ready
Reporter | ||
Updated•8 years ago
|
tracking-fennec: --- → ?
Comment 2•8 years ago
|
||
Sola, can you take this one?
Assignee: nobody → oogunsakin
tracking-fennec: ? → 29+
Assignee | ||
Comment 3•8 years ago
|
||
sure :)
Assignee | ||
Comment 4•8 years ago
|
||
Attachment #8374587 -
Flags: review?(lucasr.at.mozilla)
Assignee | ||
Comment 5•8 years ago
|
||
Flags: needinfo?(ibarlow)
Reporter | ||
Comment 6•8 years ago
|
||
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)
Assignee | ||
Comment 7•8 years ago
|
||
(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 9•8 years ago
|
||
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. :)
Assignee | ||
Comment 10•8 years ago
|
||
lol alright
Assignee | ||
Comment 11•8 years ago
|
||
Use layer drawable as background
Attachment #8374587 -
Attachment is obsolete: true
Attachment #8375042 -
Flags: feedback?(lucasr.at.mozilla)
Reporter | ||
Comment 12•8 years ago
|
||
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+
Assignee | ||
Comment 13•8 years ago
|
||
Attachment #8375042 -
Attachment is obsolete: true
Attachment #8375314 -
Flags: review?(lucasr.at.mozilla)
Reporter | ||
Comment 14•8 years ago
|
||
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+
Assignee | ||
Updated•8 years ago
|
Keywords: checkin-needed
Reporter | ||
Comment 15•8 years ago
|
||
https://hg.mozilla.org/integration/fx-team/rev/346678134a33
Keywords: checkin-needed
Comment 16•8 years ago
|
||
Backed out for mass Android test bustage. https://hg.mozilla.org/integration/fx-team/rev/516606e15ef2
Reporter | ||
Comment 17•8 years ago
|
||
Pushed a fixed patch i.e. setBackgroundDrawable() instead of setBackground() https://hg.mozilla.org/integration/fx-team/rev/360cca45a2bc
Assignee | ||
Comment 18•8 years ago
|
||
thanks Lucas :)
https://hg.mozilla.org/mozilla-central/rev/360cca45a2bc
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 30
Comment 20•8 years ago
|
||
Sola, can you request uplift to get this on aurora?
Flags: needinfo?(oogunsakin)
Assignee | ||
Comment 21•8 years ago
|
||
sure
Assignee | ||
Comment 22•8 years ago
|
||
[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)
Updated•8 years ago
|
status-firefox29:
--- → affected
status-firefox30:
--- → fixed
Updated•8 years ago
|
Attachment #8384726 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Updated•8 years ago
|
Attachment #8375314 -
Attachment is obsolete: true
Comment 24•8 years ago
|
||
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
Updated•8 years ago
|
Updated•1 year ago
|
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•