917396 - Implement top sites grid in the new 'Top Sites' panel in about:home

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: Awesomescreen, defect)

Description

[Lucas Rocha (:lucasr)]

Ian has the details ;-)

Comment 1

[Kevin Brosnan [Ex-Mozilla]]

This does not need to track 25 new about:home is 26.

Comment 2

[Sriram Ramasubramanian [:sriram]]

Attachment: Patch

Comment 3

[Lucas Rocha (:lucasr)]

Comment on attachment 806213 [details] [diff] [review]
Patch

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

Nice.

Comment 4

[Sriram Ramasubramanian [:sriram]]

http://hg.mozilla.org/projects/fig/rev/a7921e0be999

Comment 5

[Sriram Ramasubramanian [:sriram]]

Attachment: Patch: Crash

You had removed the null check for the list part. I thought that might not be needed and removed it in the grid part. But I can see a crash in tbpl. May be we need it. Adding it in both places as the grid takes the hit first in saving the list. So, if we fix this only for grid, the list will crash next :P

Comment 6

[Sriram Ramasubramanian [:sriram]]

http://hg.mozilla.org/projects/fig/rev/0d0b3b0c298f

Comment 7

[:Margaret Leibovic]

Setting to track 26, since this is part of the follow-up work to bug 917394.

Comment 8

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/a7921e0be999
https://hg.mozilla.org/mozilla-central/rev/0d0b3b0c298f

Comment 9

[Lucas Rocha (:lucasr)]

Comment on attachment 806213 [details] [diff] [review]
Patch

[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 862793 (the new about:home UI)

User impact if declined: This is part of a series of (high priority) UI changes that we'd like to get in for Firefox 26. Here's a summary:
* History and Top Sites are separate pages again
* Thumbnails in the Top Sites page (instead of Bookmarks page)
* When entering Home, Top Sites is always the default page

Testing completed (on m-c, etc.): This patch has been in our Nightly builds for almost a week now. No obvious regressions found. UI tests have been updated accordingly (see bug 917398).

Risk to taking this patch (and alternatives if risky): There is some level of risk involved here as this is not a trivial change. However, we've agreed that these changes are important enough to be worth the risk. We'd like to avoid shipping a rather big UI change in Firefox 26 to then change it all over again in the following release as this would be a too disruptive experience for our users. Given the risk involved and the fact that we're adding/changing strings, it's important to have these patches uplifted as soon as possible.

String or IDL/UUID changes made by this patch: this patch renames a few string entities to better match the new UI. We'll avoid the entity renaming where possible when uplifting (NOTE TO SHERIFFS: don't uplift this patch as is to Aurora). 

This is part of new-new-abouthome (see bug 917394)

Comment 10

[Lucas Rocha (:lucasr)]

Comment on attachment 806627 [details] [diff] [review]
Patch: Crash

[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 862793 (the new about:home UI)

User impact if declined: This is part of a series of (high priority) UI changes that we'd like to get in for Firefox 26. Here's a summary:
* History and Top Sites are separate pages again
* Thumbnails in the Top Sites page (instead of Bookmarks page)
* When entering Home, Top Sites is always the default page

Testing completed (on m-c, etc.): This patch has been in our Nightly builds for almost a week now. No obvious regressions found. UI tests have been updated accordingly (see bug 917398).

Risk to taking this patch (and alternatives if risky): There is some level of risk involved here as this is not a trivial change. However, we've agreed that these changes are important enough to be worth the risk. We'd like to avoid shipping a rather big UI change in Firefox 26 to then change it all over again in the following release as this would be a too disruptive experience for our users. Given the risk involved and the fact that we're adding/changing strings, it's important to have these patches uplifted as soon as possible.

String or IDL/UUID changes made by this patch: n/a

This is part of new-new-abouthome (see bug 917394)

Comment 11

[Lukas Blakk [:lsblakk] use ?needinfo]

Comment on attachment 806213 [details] [diff] [review]
Patch

Approving for uplift - I believe Margaret will be doing the uplift here so sheriffs don't need to worry about the string changes which are not going into uplift.

Comment 12

[Lucas Rocha (:lucasr)]

https://hg.mozilla.org/releases/mozilla-aurora/rev/ad6e4eda7e5c
https://hg.mozilla.org/releases/mozilla-aurora/rev/40b02c6fdbb2