Status

()

For bugs in Firefox Desktop, the Mozilla Foundation's web browser. For Firefox user interface issues in menus, developer tools, bookmarks, location bar, and preferences. Many Firefox bugs will either be filed here or in the Core product. (more info)

(In reply to Fred Lin [:gasolin] from comment #1)
> Dao, as comment above, PM would like to ship the compact view by default in
> 56,
>
> Is there any left work need to be done before shipping to v56?
Not that I know.

Comment on attachment 8877024[details]Bug 1369287 - shipping compact new tab page view by default;
https://reviewboard.mozilla.org/r/148358/#review152902
I don't understand this patch.
There seem to be 2 parts to this: the compact layout and the onboarding overlay.
The compact layout we can enable. I agree with Dão that it should be usable.
For the second part, the prefs listed in comment #0 suggests that we intend to disable the onboarding overlay. But this patch doesn't do that. The deps of bug 1354046 are also mostly unfixed, so I don't understand why we would want to ship this. Certainly the onboarding overlay is still empty and useless in my nightly. Turning it on off-nightly seems premature.
Furthermore, the code as-is suggests that the browser.onboarding.enabled pref is already set to true (it's not part of this patch, but the context in this file has it set to true), but the code is ifdef'd out to only be available on Nightly (ifdefs removed in this patch). How do the pref and the non-existent code interact on 55 now that that's off-nightly? Should the pref also be ifdef'd for 55?
So, to summarize, while I would r+ only the 2 pref changes to swap over to a compact layout for tiles, I think we should postpone making a decision about shipping the onboarding overlay in 56 to later in the cycle when we are confident that all the relevant bits have landed and we are happy to ship the overlay in its then-current state.

this patch is target for 56 so it won't affect 55.
`browser.onboarding.disabled=true` in Firefox.js is moved to `browser.onboarding.enabled=true` which is on by default in nightly. So we don't need to `enable` it in firefox.js.
There are several overlay tours waiting in queue and will be landed this week. I'll wait until they are ready and send review again.

Though onboarding has great progress and it will be more stable after ALL Hands.
I think its still worth to ship compact view earlier and see if there's any issue around that.
So I plan to file another bug and send PR once we feel confident to release onboarding overlay to all versions.

I have a wide screen (1920 x 1080). This compact newpage looks so empty and the thumbnails are so tiny that they become useless because I can barely see what's in the image. I reverted the pref immediately. Has the UX team really thought about this?

(In reply to Oriol [:Oriol] from comment #18)
> I have a wide screen (1920 x 1080). This compact newpage looks so empty and
> the thumbnails are so tiny that they become useless because I can barely see
> what's in the image. I reverted the pref immediately. Has the UX team really
> thought about this?
FWIW, I agree the thumbnails are too small, but this is what I was asked to implement. Personally, I have zoomed in on the new tab page to 133%.

(In reply to Oriol [:Oriol] from comment #18)
> I have a wide screen (1920 x 1080). This compact newpage looks so empty and
> the thumbnails are so tiny that they become useless because I can barely see
> what's in the image. I reverted the pref immediately. Has the UX team really
> thought about this?
Oriol thanks for the report! I forwarded your concern to activity stream team (who designed that UI) and wish they will help find out the best result.

Some concerns were raised about shipping this experience in 56 given that Activity Stream was coming in 57.
In particular: we'd be changing the experience and then changing it again one release later with different methods of pinning and controlling the content. It would be better to change it once given how frequently this page is shown. In addition, Activity Stream would be setting new top site defaults, so it didn't make sense to ship a new set in 56 (which hasn't been defined per locale).
As a result, I am asking that this change be backed out such that we revert to the previous newtab implementation.
The onboarding tour overlay and notifications would live on top of that implementation (non-compact).
Fred, are you able to make that change?

(In reply to Peter Dolanjski [:pdol] from comment #22)
> Some concerns were raised about shipping this experience in 56 given that
> Activity Stream was coming in 57.
> In particular: we'd be changing the experience and then changing it again
> one release later with different methods of pinning and controlling the
> content. It would be better to change it once given how frequently this page
> is shown.
After the backout, I still see something different than before.
I have a newtab page with 4x3 thumbnails and I need to scroll to see the bottom four.
Previously, the page contained 4x2 thumbnails and I didn't have to scroll at all.

(In reply to Marco Castelluccio [:marco] from comment #25)
> (In reply to Peter Dolanjski [:pdol] from comment #22)
> > Some concerns were raised about shipping this experience in 56 given that
> > Activity Stream was coming in 57.
> > In particular: we'd be changing the experience and then changing it again
> > one release later with different methods of pinning and controlling the
> > content. It would be better to change it once given how frequently this page
> > is shown.
>
> After the backout, I still see something different than before.
> I have a newtab page with 4x3 thumbnails and I need to scroll to see the
> bottom four.
> Previously, the page contained 4x2 thumbnails and I didn't have to scroll at
> all.
The default number of rows was always 3, so if you had 2 rows then you customized that in about:config. You're just seeing this because it switched to 2 (which meant your value in user prefs got blown away because it matched the default) and now back to 3, so now you got migrated to the 'new' default. That's unfortunate, but not something we can do an awful lot about, and it won't hit anyone other than nightly users (not off-nightly because it never changed there) who had this exact matching pref.

(In reply to :Gijs (no reviews, mostly out until Jul 17) from comment #26)
> (In reply to Marco Castelluccio [:marco] from comment #25)
> > (In reply to Peter Dolanjski [:pdol] from comment #22)
> > > Some concerns were raised about shipping this experience in 56 given that
> > > Activity Stream was coming in 57.
> > > In particular: we'd be changing the experience and then changing it again
> > > one release later with different methods of pinning and controlling the
> > > content. It would be better to change it once given how frequently this page
> > > is shown.
> >
> > After the backout, I still see something different than before.
> > I have a newtab page with 4x3 thumbnails and I need to scroll to see the
> > bottom four.
> > Previously, the page contained 4x2 thumbnails and I didn't have to scroll at
> > all.
>
> The default number of rows was always 3, so if you had 2 rows then you
> customized that in about:config. You're just seeing this because it switched
> to 2 (which meant your value in user prefs got blown away because it matched
> the default) and now back to 3, so now you got migrated to the 'new'
> default. That's unfortunate, but not something we can do an awful lot about,
> and it won't hit anyone other than nightly users (not off-nightly because it
> never changed there) who had this exact matching pref.
I didn't actually remember the number of thumbnails I had before, I got it by running a Nightly from 2017-04-04 with a clean profile, where the default was 4x2.

To be clear, "browser.newtabpage.rows" is 3 and "browser.newtabpage.columns" is 5 in the previous Nightly, and there are 4x2 thumbnails in about:newtab.
In the new Nightly, the prefs are the same, but there are 4x3 thumbnails in about:newtab.

(In reply to Marco Castelluccio [:marco] from comment #28)
> To be clear, "browser.newtabpage.rows" is 3 and "browser.newtabpage.columns"
> is 5 in the previous Nightly, and there are 4x2 thumbnails in about:newtab.
> In the new Nightly, the prefs are the same, but there are 4x3 thumbnails in
> about:newtab.
File a separate bug and needinfo with an accurate description of (a) what the states of the prefs were and (b) what the problem is in your view. Right now I don't understand anymore from the repeated comments with the different information, and in any case it seems like it might not have been caused by this bug considering the backout was pretty straightforward, so continuing here doesn't seem like it makes sense.