Start Page (about:home) could be better on tablets

VERIFIED FIXED in Firefox 15

Status

()

Firefox for Android
Theme and Visual Design
VERIFIED FIXED
6 years ago
a year ago

People

(Reporter: lucasr, Assigned: mcomella)

Tracking

(Blocks: 1 bug)

Trunk
Firefox 17
All
Android
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite -

Firefox Tracking Flags

(firefox15 verified, firefox16 verified, firefox17 verified, fennec15+)

Details

Attachments

(9 attachments, 19 obsolete attachments)

565.68 KB, image/jpeg
Details
575.77 KB, image/jpeg
Details
584.34 KB, image/jpeg
Details
5.72 KB, patch
mcomella
: review+
Details | Diff | Splinter Review
3.63 KB, patch
sriram
: review+
Details | Diff | Splinter Review
598.53 KB, image/jpeg
Details
11.89 KB, patch
mcomella
: review+
Details | Diff | Splinter Review
28.94 KB, patch
mcomella
: review+
Details | Diff | Splinter Review
4.96 KB, patch
mcomella
: review+
Details | Diff | Splinter Review
(Reporter)

Description

6 years ago
We could do a more interesting about:home for the larger screen on tablets.
(Reporter)

Updated

6 years ago
Blocks: 655762
Keywords: uiwanted
(Reporter)

Updated

6 years ago
Duplicate of this bug: 688442
Created attachment 562134 [details]
rough mockup

This is a ROUGH work in progess mockup of a potential first version of about:home for tablets. Functionally, it is the same as the phone version, but giving the CSS a good looking over to make it feel more tablet friendly. 

I'm not done yet, but wanted to post something to gauge people's reaction so far.
Assignee: nobody → sriram
Summary: about:home could be better on tablets → Start Page (about:home) could be better on tablets
Note that for the Sync Setup Improvements feature, we're adding a consistent way to discover Sync across all devices, using "Set up Sync" and "Pair a Device" links on the home tabs. This already landed in mobile in bug 675820.
Created attachment 565356 [details] [diff] [review]
Rough WIP

This patch adds a new file for honeycomb and makes the changes as per the mockup.
The padding and margin values are not exact as the mockup yet.
The width of the section halves when in landscape mode -- however, they don't switch to two different columns.

I feel, there should be more concrete decisions on what sections should be where, how each section will be when they are at their maximum and minimum heights.
Ian, we should probably sync up on the designs of about:home on desktop and the start page on tablets.  Overall, we could probably use the same design on both with a few tweaks (bigger targets on tablet, more prominence for sync, etc).  I'm working on iteration 2 of about:home right now, but here's iteration 1: http://people.mozilla.com/~jboriss/specs/home_tab_first_iteration_spec1.png.  If you think tabs from last time is very important, perhaps we could display those at the bottom where the launch targets are in that spec.  Or, maybe we should just wait until the second iteration of desktop about:home is further along so it will be easier to add tablet-specific content
Keywords: uiwanted
Hi Boriss, that start page is looking great, I look forward to seeing the next round of designs. I'll ping you this week to show you what we're thinking about on tablets. 

I think a key distinction on mobile will be to offer lots of ways to quickly and seamlessly get to your content without having to type anything. 

Currently we show (or are planning to show)
- Sync Setup
- Top Sites
- Add-ons
- Session Restore
- Remote Tabs

And down the road this could grow to include things like:
- Reading List
- Social
- Apps
- Pancake-style content stream

I will keep you posted, and share some more designs later this week :)

Updated

5 years ago
Assignee: sriram → nobody
tracking-fennec: --- → ?
status-firefox15: --- → affected
status-firefox16: --- → affected
status-firefox17: --- → affected
Component: General → Theme and Visual Design
Product: Fennec → Firefox for Android
tracking-fennec: ? → 15+
sriram and mcomella, new mockups for about:home can be found here: http://www.flickr.com/photos/61892693@N03/sets/72157630730911182/detail/

And measurements for how to scale thumbnails across device sizes can be found here http://cl.ly/1s351T3u2o15

mcomella, I would suggest syncing up with Sriram on how to approach the layout changes -- there is probably a lot of styling that we can copy over from the current about:home implementation on phones.
(Assignee)

Updated

5 years ago
Assignee: nobody → michael.l.comella
Status: NEW → ASSIGNED
(Assignee)

Comment 8

5 years ago
Created attachment 645859 [details] [diff] [review]
Top Sites Change WIP

In addition to the XML changes, the top sites code needs to change. There are a few ways to do this. This is one of them.

I chose this way since it keeps the new code in the same place and requires the fewest changes to the existing code. I dislike this change because in order for TopSitesGridView to be inflated, it needs to be a static inner class (or at least, I believe that's the issue), meaning the mTopSites... etc. variables need to also be static. This assumes that we cannot have two simultaneous screen configurations with two AboutHomeContent instances... which seems like a reasonable assumption, given current devices, but it is not entirely future proof.

An alternative is to sub class AboutHomeContent for each layout type and have the mTopSites... variables get reset in those (also changing their access levels). This would work well to abstract things but it would be gross to have a new class for each layout. This also means the gecko_app.xml.in needs a new file for each layout to construct the appropriate AboutHomeContent (or alternatively, AboutHomeContent needs to wrap an appropriate subclass instance).

Is my solution ideal? Am I overthinking this?
Attachment #645859 - Flags: feedback?(sriram)
(Assignee)

Updated

5 years ago
Attachment #562134 - Attachment is obsolete: true
(Assignee)

Updated

5 years ago
Attachment #565356 - Attachment is obsolete: true
Comment on attachment 645859 [details] [diff] [review]
Top Sites Change WIP

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

There is a better way to do this.
Please refer to values/dimens.xml and see how we use them.
Create appropriate integers.xml and get values from that.
Attachment #645859 - Flags: feedback?(sriram) → feedback-
(Assignee)

Comment 10

5 years ago
Created attachment 645901 [details] [diff] [review]
Patch 01: Integer Resources

Moved to integer resources.
Attachment #645859 - Attachment is obsolete: true
Attachment #645901 - Flags: feedback?(sriram)
Comment on attachment 645901 [details] [diff] [review]
Patch 01: Integer Resources

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

Looks good overall.
I usually don't like doing this:
Resources res = getResources();
and then using it. Better use getResources() directly. It's not costly.

values-xlarge-land-v11 and values-land-v11 have the same values. You can axe values-xlarge-land-v11/. It will default to values-xlarge-v11/integers.xml.

And values-land/integers.xml overrides only one value. Please have only that value there.

setTopSitesConstants() can be moved further down -- to just before it's being used for the very first time.
Attachment #645901 - Flags: feedback?(sriram) → feedback+
(Assignee)

Comment 12

5 years ago
Created attachment 645916 [details] [diff] [review]
01a: Integer Resources

Fixed the feedback.

(In reply to Sriram Ramasubramanian [:sriram] from comment #11)
> Comment on attachment 645901 [details] [diff] [review]
>
> setTopSitesConstants() can be moved further down -- to just before it's
> being used for the very first time.

Not sure what you meant by this so I moved both the declaration and the first call lower.
Attachment #645901 - Attachment is obsolete: true
(Assignee)

Comment 13

5 years ago
Created attachment 646025 [details]
02: Created 10" tablet layout.

I split the 10" layout into two columns with minimal copy-paste. Let me know what you think.

Also, abouthome_content_footer is a HORRIBLE name. I know. It will be changed.

Besides the TODOs in the code, here are some things that still need to happen:
* The layout does not change on rotation for the 10" tab. I tried adding requestLayout() to onConfigurationChanged but it did not seem to work.
* The padding between thumbnails needs to be adjusted.
* The thumbnail sizes needs to be adjusted (I will do this last as we spoke about)

There are also some implicit changes I noticed in the mocks that did not have specifications – font sizes, margins, and padding changes between different sized devices (for example, look at the thumbnail alignment with the device edge in relation to the "Top Sites" header in a portrait orientation). Should I include these changes?
Attachment #646025 - Flags: feedback?(sriram)
(Assignee)

Comment 14

5 years ago
I also was not sure where the sync container should go.
(Assignee)

Comment 15

5 years ago
Created attachment 646030 [details] [diff] [review]
02a: Created 10" tablet layout.

Forgot to qref properly.
Attachment #646025 - Attachment is obsolete: true
Attachment #646025 - Flags: feedback?(sriram)
Attachment #646030 - Flags: feedback?(sriram)
(Assignee)

Comment 16

5 years ago
Created attachment 646199 [details]
10'' Tab landscape after patch 02a.
(Assignee)

Updated

5 years ago
Attachment #646199 - Attachment description: 10'' Tab landscape after match 02a. → 10'' Tab landscape after patch 02a.
Comment on attachment 646030 [details] [diff] [review]
02a: Created 10" tablet layout.

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

We don't need to split the abouthome_content.xml into more resuable stuff. Because <include/> is costly.
On seeing the mockups, this is what I feel, everything except 10" landscape has one linear layout -- this will be satisfied by abouthome_content.xml
For the case of 10" landscape -- layout-xlarge-land-v11/ would satisfy.
Post a patch with this change -- just changing the layout as per mockup in layout-xlarge-land-v11/abouthome_content.xml(.in maybe)

The next patch would add necessary padding - both for entire about:home in 10", and between thumbnails. This can all be added in dimens.xml in various places and be used in layouts.

Please don't change the thumbnail size as of now.
Attachment #646030 - Flags: feedback?(sriram) → feedback-
Comment on attachment 645916 [details] [diff] [review]
01a: Integer Resources

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

This looks good to me.
Attachment #645916 - Flags: review+
(Assignee)

Comment 19

5 years ago
Created attachment 646692 [details] [diff] [review]
02b: Created 10" tablet layout.

Updated the layout as per your comment.

I chose not to move the sync box as it's a little messy – it adds relative layout parameters in the code (AboutHomeContent.updateSyncLayout() – line 242), which would then be dependent on orientation and screen size.
Attachment #646030 - Attachment is obsolete: true
Attachment #646199 - Attachment is obsolete: true
Attachment #646692 - Flags: feedback?(sriram)
(Assignee)

Comment 20

5 years ago
> I chose not to move the sync box...

I think it would be more appropriate to do in a separate patch.
(Assignee)

Comment 21

5 years ago
Created attachment 646694 [details]
about:home, 10" landscape, after patch 02b.
(Assignee)

Comment 22

5 years ago
Created attachment 646779 [details] [diff] [review]
02c: Cleaned about:home and created 10" tablet layout.

As talked about in person, cleaned up about:home code and thus created a cleaner 10" tablet layout.
Attachment #646692 - Attachment is obsolete: true
Attachment #646694 - Attachment is obsolete: true
Attachment #646692 - Flags: feedback?(sriram)
Attachment #646779 - Flags: feedback?(sriram)
(Assignee)

Comment 23

5 years ago
Created attachment 646781 [details]
about:home, 10" landscape, after patch 02c.
(Assignee)

Updated

5 years ago
Attachment #646779 - Attachment description: 02b: Cleaned about:home and created 10" tablet layout. → 02c: Cleaned about:home and created 10" tablet layout.
(Assignee)

Comment 24

5 years ago
Created attachment 646783 [details] [diff] [review]
02d: Cleaned about:home and created 10" tablet layout.
Attachment #646779 - Attachment is obsolete: true
Attachment #646779 - Flags: feedback?(sriram)
Attachment #646783 - Flags: feedback?(sriram)
(Assignee)

Comment 25

5 years ago
Created attachment 646785 [details]
about:home, 10" landscape, after patch 02d.
Attachment #646781 - Attachment is obsolete: true
(Assignee)

Updated

5 years ago
Attachment #646783 - Attachment is patch: true
(Assignee)

Comment 26

5 years ago
Note that the screen still does not use the new resources on a configuration change (so starting in landscape and rotating will continue to use landscape about:home). I believe the View needs to be reinflated in onConfigurationChanged(). I will get to that ASAP.
Hi mcomella, let me know when you feel like you are ready for a UI review. 

Happy to jump in whenever you're ready :)
Comment on attachment 646783 [details] [diff] [review]
02d: Cleaned about:home and created 10" tablet layout.

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

Overall looks good.
Few changes are needed:
1. android:layout_width="0dp" <--- use "wrap_content". It does the same job.
2. Instead of ".4", please use "0.4", looks better. (same for 0.6)
3. "match_parent" <-- this will crash in froyo. Please use "fill_parent".
4. The code logic has removed the need for "no_top_sites_text". Please remove it from both the XMLs.

And didn't we decide on a marginTop for the "top_sites_title"? How does that work without a marginTop?
Attachment #646783 - Flags: feedback?(sriram) → feedback+
(Assignee)

Comment 29

5 years ago
Created attachment 647292 [details] [diff] [review]
02e: Cleaned about:home and created 10" tablet layout.

Note: had f+.

(In reply to Sriram Ramasubramanian [:sriram] from comment #28)
> Few changes are needed:
> 1. android:layout_width="0dp" <--- use "wrap_content". It does the same job.
As per talk in real-life, wrap_content does not work properly. Keeping 0dp.

> 2. Instead of ".4", please use "0.4", looks better. (same for 0.6)
Done.

> 3. "match_parent" <-- this will crash in froyo. Please use "fill_parent".
Done (additionally, bug 778862 to clean up additional uses).

> 4. The code logic has removed the need for "no_top_sites_text". Please
> remove it from both the XMLs.
Discussion for bug 778811.

> And didn't we decide on a marginTop for the "top_sites_title"? How does that
> work without a marginTop?
I don't remember discussing this. The element above it (the Firefox logo) uses marginBottom. Do you want this changed?

Things still to be changed (additional patches):
* Padding between thumbnails
* about:home doesn't change the view resources on orientation change. These must be re-inflated by hand.
Attachment #646783 - Attachment is obsolete: true
Attachment #647292 - Flags: review?(sriram)
Comment on attachment 647292 [details] [diff] [review]
02e: Cleaned about:home and created 10" tablet layout.

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

This looks good to me.
Attachment #647292 - Flags: review?(sriram) → review+
(Assignee)

Comment 31

5 years ago
Created attachment 647359 [details] [diff] [review]
03: Added padding for tablet layout.
Attachment #647359 - Flags: review?(sriram)
(Assignee)

Comment 32

5 years ago
Created attachment 647360 [details]
about:home, 10" landscape after patch 03

Note: The FF icon (top-right) is slightly cut-off because the original graphic is created this way. ibarlow's mocks use the full image.
(Assignee)

Comment 33

5 years ago
Created attachment 647387 [details] [diff] [review]
03a: Added padding for tablet layout.

Added padding for portrait mode too.
Attachment #647359 - Attachment is obsolete: true
Attachment #647359 - Flags: review?(sriram)
Attachment #647387 - Flags: review?(sriram)
(Assignee)

Comment 34

5 years ago
Created attachment 647388 [details]
about:home, 10" portrait after patch 03a
(Assignee)

Updated

5 years ago
Attachment #647388 - Attachment description: about:home, 10" portrait after patch 03 → about:home, 10" portrait after patch 03a
(Assignee)

Comment 35

5 years ago
I added "onDestroy(); init();" to the onConfigurationChanged() code and it doesn't let me re-inflate because AboutHomeContent (as a Scrollview) can only have one direct child view.

I changed it to "onDestroy(); removeAllViews(); init();" and it does not crash on rotation, however, the child views do not repopulate their content properly. The top sites title, browse all your top sites text (which is positioned properly), and occasionally the sync box are all the elements that show. I'll keep looking into it but I was wondering if you had any idea why this would be.
(Assignee)

Comment 36

5 years ago
Created attachment 647409 [details] [diff] [review]
04: kTopSitesItemHeight now uses a dimen constant.

Updated the value to 106 also.
Attachment #647409 - Flags: review?(sriram)
(Reporter)

Comment 37

5 years ago
(In reply to Michael Comella (:mcomella) from comment #35)
> I added "onDestroy(); init();" to the onConfigurationChanged() code and it
> doesn't let me re-inflate because AboutHomeContent (as a Scrollview) can
> only have one direct child view.
> 
> I changed it to "onDestroy(); removeAllViews(); init();" and it does not
> crash on rotation, however, the child views do not repopulate their content
> properly. The top sites title, browse all your top sites text (which is
> positioned properly), and occasionally the sync box are all the elements
> that show. I'll keep looking into it but I was wondering if you had any idea
> why this would be.

Drive-by comment: calling API callbacks (especially life-cycle ones) directly is very rarely a good idea... Factor out the code into separate private methods and call them appropriately where needed.
Work in here might also want to consider bug 778914.
Comment on attachment 647387 [details] [diff] [review]
03a: Added padding for tablet layout.

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

The approach is good. But I would like following changes for the sake of symmetric behavior.
There is a "column_spacing". Instead, try applying the margin to the two vertical columns (instead of the parent).
Please kill the new folder values-xlarge-land-v11. (Based on above comment, we dont want it anymore. But still, it can always default to values-xlarge-v11/ folder if the dimen name is different).

So, if you could rename the dimens to:
abouthome_gutter_small - value of 20dp and
abouthome_gutter_large - value of 40dp, it should be good.
Attachment #647387 - Flags: review?(sriram) → review-
Comment on attachment 647409 [details] [diff] [review]
04: kTopSitesItemHeight now uses a dimen constant.

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

kTopStiesXxx.. will become mTopSitesXxx.
mDisplayDensity is no longer need.
It's better to set the value of mTopSitesXxx inside the constructor.
Please make these changes.
Attachment #647409 - Flags: review?(sriram) → review-
(Assignee)

Comment 41

5 years ago
Created attachment 647604 [details] [diff] [review]
03b: Added padding for tablet layout.
Attachment #647387 - Attachment is obsolete: true
Attachment #647604 - Flags: review?(sriram)
Comment on attachment 647604 [details] [diff] [review]
03b: Added padding for tablet layout.

Perfect! :)
Though "abouthome_gutter_small" is not used in values/, please add it there too, for the sake of completion. It will be 0dp.
Attachment #647604 - Flags: review?(sriram) → review+
(Assignee)

Comment 43

5 years ago
Created attachment 647607 [details] [diff] [review]
03c: Added padding for tablet layout.

Moved r+. Added abouthome_gutter_small to .../res/values/dimens.xml.
Attachment #647604 - Attachment is obsolete: true
Attachment #647607 - Flags: review+
(Assignee)

Comment 44

5 years ago
Created attachment 647615 [details] [diff] [review]
04a: TopSitesItemHeight uses a constant and is retrieved dynamically
Attachment #647409 - Attachment is obsolete: true
Attachment #647615 - Flags: review?(sriram)
(Assignee)

Updated

5 years ago
Attachment #647615 - Attachment description: 04a: TopSitesItemHeight is now a constant and retrieved dynamically. → 04a: TopSitesItemHeight uses a constant and is retrieved dynamically
Comment on attachment 647615 [details] [diff] [review]
04a: TopSitesItemHeight uses a constant and is retrieved dynamically

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

Looks good to me.
Attachment #647615 - Flags: review?(sriram) → review+
(Assignee)

Comment 46

5 years ago
Created attachment 647774 [details] [diff] [review]
05: about:home correctly adjusts itself on configuration change.
Attachment #647774 - Flags: review?(sriram)
(Assignee)

Comment 47

5 years ago
Created attachment 647776 [details]
about:home, 10" landscape after patch 05 w/ synced tabs
Comment on attachment 647774 [details] [diff] [review]
05: about:home correctly adjusts itself on configuration change.

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

This looks good to me. Few changes:
1. Let's call init() as inflate() -- the one that inflates the view
2. The one that does a much of findViewById -- as init()
Attachment #647774 - Flags: review?(sriram) → review+
(In reply to Sriram Ramasubramanian [:sriram] from comment #48)
> Comment on attachment 647774 [details] [diff] [review]
> 05: about:home correctly adjusts itself on configuration change.
> 
> Review of attachment 647774 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> This looks good to me. Few changes:
> 1. Let's call init() as inflate() -- the one that inflates the view
> 2. The one that does a much of findViewById -- as init()

And when you change, you might need to change in BrowserApp as well.
(Assignee)

Comment 50

5 years ago
Created attachment 648091 [details] [diff] [review]
01b: Integer Resources

Rebase to trunk. Moved r+.
Attachment #645916 - Attachment is obsolete: true
Attachment #648091 - Flags: review+
(Assignee)

Comment 51

5 years ago
Created attachment 648092 [details] [diff] [review]
02f: Cleaned about:home and created 10" tablet layout.

Rebase to trunk. Moved r+.
Attachment #647292 - Attachment is obsolete: true
Attachment #648092 - Flags: review+
(Assignee)

Comment 52

5 years ago
Created attachment 648097 [details] [diff] [review]
05a: about:home correctly adjusts itself on configuration change.

Made method name changes, moved r+. I have a bad build so I can't test it. I will mark checkin-needed if everything is okay afterward.
Attachment #647774 - Attachment is obsolete: true
Attachment #648097 - Flags: review+
(Assignee)

Updated

5 years ago
Keywords: checkin-needed
https://hg.mozilla.org/integration/mozilla-inbound/rev/81a6affeaebc
https://hg.mozilla.org/integration/mozilla-inbound/rev/584d61c94a1c
https://hg.mozilla.org/integration/mozilla-inbound/rev/080333495957
https://hg.mozilla.org/integration/mozilla-inbound/rev/ac4872c6b21a
https://hg.mozilla.org/integration/mozilla-inbound/rev/88e900fd365e
Flags: in-testsuite-
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/81a6affeaebc
https://hg.mozilla.org/mozilla-central/rev/584d61c94a1c
https://hg.mozilla.org/mozilla-central/rev/080333495957
https://hg.mozilla.org/mozilla-central/rev/ac4872c6b21a
https://hg.mozilla.org/mozilla-central/rev/88e900fd365e
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 17
Verified via Galaxy Tab 10.1 (Android 4.0.3)
Status: RESOLVED → VERIFIED
status-firefox17: affected → verified

Updated

5 years ago
Depends on: 780367
Depends on: 780481
Depends on: 780486
Comment on attachment 648091 [details] [diff] [review]
01b: Integer Resources

[Approval Request Comment]
Bug caused by (feature/regressing bug #): New UI
User impact if declined: No optimized about:home for tablets
Testing completed (on m-c, etc.): Landed in m-c on 08/02
Risk to taking this patch (and alternatives if risky): None.
String or UUID changes made by this patch: None.
Attachment #648091 - Flags: approval-mozilla-beta?
Attachment #648091 - Flags: approval-mozilla-aurora?
Comment on attachment 648092 [details] [diff] [review]
02f: Cleaned about:home and created 10" tablet layout.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): New UI
User impact if declined: No optimized about:home for tablets
Testing completed (on m-c, etc.): Landed in m-c on 08/02
Risk to taking this patch (and alternatives if risky): None.
String or UUID changes made by this patch: None.
Attachment #648092 - Flags: approval-mozilla-beta?
Attachment #648092 - Flags: approval-mozilla-aurora?
Comment on attachment 648097 [details] [diff] [review]
05a: about:home correctly adjusts itself on configuration change.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): New UI
User impact if declined: No optimized about:home for tablets
Testing completed (on m-c, etc.): Landed in m-c on 08/02
Risk to taking this patch (and alternatives if risky): None.
String or UUID changes made by this patch: None.
Attachment #648097 - Flags: approval-mozilla-beta?
Attachment #648097 - Flags: approval-mozilla-aurora?
Comment on attachment 647607 [details] [diff] [review]
03c: Added padding for tablet layout.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): New UI
User impact if declined: No optimized about:home for tablets
Testing completed (on m-c, etc.): Landed in m-c on 08/02
Risk to taking this patch (and alternatives if risky): None.
String or UUID changes made by this patch: None.
Attachment #647607 - Flags: approval-mozilla-beta?
Attachment #647607 - Flags: approval-mozilla-aurora?
Comment on attachment 647615 [details] [diff] [review]
04a: TopSitesItemHeight uses a constant and is retrieved dynamically

[Approval Request Comment]
Bug caused by (feature/regressing bug #): New UI
User impact if declined: No optimized about:home for tablets
Testing completed (on m-c, etc.): Landed in m-c on 08/02
Risk to taking this patch (and alternatives if risky): None.
String or UUID changes made by this patch: None.
Attachment #647615 - Flags: approval-mozilla-beta?
Attachment #647615 - Flags: approval-mozilla-aurora?
Attachment #647607 - Flags: approval-mozilla-beta?
Attachment #647607 - Flags: approval-mozilla-beta+
Attachment #647607 - Flags: approval-mozilla-aurora?
Attachment #647607 - Flags: approval-mozilla-aurora+
Attachment #647615 - Flags: approval-mozilla-beta?
Attachment #647615 - Flags: approval-mozilla-beta+
Attachment #647615 - Flags: approval-mozilla-aurora?
Attachment #647615 - Flags: approval-mozilla-aurora+
Attachment #648091 - Flags: approval-mozilla-beta?
Attachment #648091 - Flags: approval-mozilla-beta+
Attachment #648091 - Flags: approval-mozilla-aurora?
Attachment #648091 - Flags: approval-mozilla-aurora+
Attachment #648092 - Flags: approval-mozilla-beta?
Attachment #648092 - Flags: approval-mozilla-beta+
Attachment #648092 - Flags: approval-mozilla-aurora?
Attachment #648092 - Flags: approval-mozilla-aurora+
Attachment #648097 - Flags: approval-mozilla-beta?
Attachment #648097 - Flags: approval-mozilla-beta+
Attachment #648097 - Flags: approval-mozilla-aurora?
Attachment #648097 - Flags: approval-mozilla-aurora+
Approving for branches and adding qawanted - can QA please check once this has landed on Beta, against our shipping Android locales to make sure that this new layout doesn't break on any languages?
Keywords: qawanted
(Assignee)

Updated

5 years ago
Keywords: checkin-needed
(Assignee)

Comment 62

5 years ago
Please note these patches should be checked in, then the patches in bug 780481 and bug 780367 (these last two can be in any order).
None of these applied to cleanly to aurora or beta, but the unbitrotting was pretty mechanical. That said, please look them over to make sure I didn't mess anything up.

https://hg.mozilla.org/releases/mozilla-aurora/rev/8738a36b2a9e
https://hg.mozilla.org/releases/mozilla-aurora/rev/8518a8edd111
https://hg.mozilla.org/releases/mozilla-aurora/rev/76cff107b031
https://hg.mozilla.org/releases/mozilla-aurora/rev/b6a50c80dad6
https://hg.mozilla.org/releases/mozilla-aurora/rev/d54a1c9b1f5c

https://hg.mozilla.org/releases/mozilla-beta/rev/20791a8e92ab
https://hg.mozilla.org/releases/mozilla-beta/rev/3255b76324ca
https://hg.mozilla.org/releases/mozilla-beta/rev/75827bc5b1f7
https://hg.mozilla.org/releases/mozilla-beta/rev/729013a15c82
https://hg.mozilla.org/releases/mozilla-beta/rev/7567e848fd44
status-firefox15: affected → fixed
status-firefox16: affected → fixed
Keywords: checkin-needed
Had to push a bustage fix for part 5 to both branches. Based on code inspection, I'm assuming this was the correct fix.

https://hg.mozilla.org/releases/mozilla-aurora/rev/3b53726859fd
https://hg.mozilla.org/releases/mozilla-beta/rev/2de209a396f2

For posterity, the build error was:

/builds/slave/m-beta-andrd/build/mobile/android/base/AboutHomeContent.java:155: cannot find symbol
symbol  : variable mContext
location: class org.mozilla.gecko.AboutHomeContent
        mInflater = (LayoutInflater) mContext.getSystemService(Context.LAYOUT_INFLATER_SERVICE);
Well, so much for that idea. Still busted. Please post branch-specific patches. Backed out from both branches.

https://hg.mozilla.org/releases/mozilla-aurora/rev/1b8af79d5194
https://hg.mozilla.org/releases/mozilla-beta/rev/09e2ae313229
status-firefox15: fixed → affected
status-firefox16: fixed → affected
So, are all patches backed out?
Yes, per the commit message on the backouts.
Pushed to beta:
https://hg.mozilla.org/releases/mozilla-beta/rev/e5c1e2ad1612
https://hg.mozilla.org/releases/mozilla-beta/rev/7fbe3918780d
https://hg.mozilla.org/releases/mozilla-beta/rev/40344ba73955
https://hg.mozilla.org/releases/mozilla-beta/rev/aeec73113fe7
https://hg.mozilla.org/releases/mozilla-beta/rev/cc3973e2be29
status-firefox15: affected → fixed
And to aurora:
https://hg.mozilla.org/releases/mozilla-aurora/rev/79c412fa3945
https://hg.mozilla.org/releases/mozilla-aurora/rev/1f826579aae3
https://hg.mozilla.org/releases/mozilla-aurora/rev/6958061dc711
https://hg.mozilla.org/releases/mozilla-aurora/rev/0ef5b2e31835
https://hg.mozilla.org/releases/mozilla-aurora/rev/3a7b67da4faa
status-firefox16: affected → fixed

Updated

5 years ago
Keywords: qawanted
New about:home page is present on:

Firefox Mobile 15 / Firefox Mobile 16.0b5 
Asus Transformer TF101 (Android 4.0.3)

Marking as verified.
status-firefox15: fixed → verified
status-firefox16: fixed → verified
You need to log in before you can comment on or make changes to this bug.