1180387 - New Tab Page doesn't show websites (wrong number of rows or lines)

Status: RESOLVED FIXED

(Firefox :: New Tab Page, defect)

Description

[Moltres Rider]

Attachment: see second row of websites missing, they appeared for a second then disappeared

User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:39.0) Gecko/20100101 Firefox/39.0
Build ID: 20150630154324

Steps to reproduce:

open web browser after Firefox updated
open the new tab page


Actual results:

the websites that list as my frequent websites appear and the second row of websites disappear IMMEDIATELY seconds after they appear. bug did NOT occur in ANY previous version of Firefox


Expected results:

the thumbnails should stay on the new tab page

Comment 1

[Jan Manthay]

This is also reported by multiple people on the german camp-firefox forum.

Comment 2

[Liz Henry (:lizzard) (relman/hg->git project)]

Similar reports appear on SUMO: 

https://support.mozilla.org/en-US/questions/1070178?utm_campaign=questions-reply&utm_medium=email&utm_source=notification

https://support.mozilla.org/de/questions/1070115

Comment 3

[the-edmeister]

This issue with the new Tab page having fewer rows (and maybe fewer columns) than were seen in Firefox 38 and prior seems to have started with Firefox 39. Plus some users have mentioned that zooming out restore the columns and rows layout they were seeing with Firefox 38.0.5 and prior.

Comment 4

[Moltres Rider]

(In reply to the-edmeister from comment #3)
> This issue with the new Tab page having fewer rows (and maybe fewer columns)
> than were seen in Firefox 38 and prior seems to have started with Firefox
> 39. Plus some users have mentioned that zooming out restore the columns and
> rows layout they were seeing with Firefox 38.0.5 and prior.

I tried zooming out but it always later reverts to its original size, closing Firefox or not, it eventually returns to normal size and less rows. it WON'T stick!

Comment 5

[neph]

I'm experiencing the same problem but for me the missing tiles don't even show for a split second, when opening a new tab, they simply don't get displayed at all.
With FF 38 I had 3 rows and 4 columns of tiles but with FF 39 there are only 2 rows, so 8 tiles in total now. Setting the zoom to 80% shows the missing row but it also decreases the overall font size and like other persons already said here, it always reverts back to 100%, when you close the tab.
In about:config 
"browser.newtabpage.columns" is set to 5
and
"browser.newtabpage.rows" is set to 3.

Comment 6

[Loic]

[Tracking Requested - why for this release]:

Bug reported on the French community board too:
https://forums.mozfr.org/viewtopic.php?f=5&t=125113

Comment 7

[the-edmeister]

Could that be happening due to the pref - browser.newtab.preload - set to True?
Is that a new pref in Firefox 39 or was that pref default value changed in Fx39?

Whichever, toggling that pref to false seems to revert the the behavior that was in Firefox 38.0.5.
https://support.mozilla.org/en-US/questions/1070156

Comment 8

[penguino]

'browser.newtab.preload' is not the problem.

Comment 9

[Moin Shaikh]

(In reply to neph from comment #5)
> I'm experiencing the same problem but for me the missing tiles don't even
> show for a split second, when opening a new tab, they simply don't get
> displayed at all.
> With FF 38 I had 3 rows and 4 columns of tiles but with FF 39 there are only
> 2 rows, so 8 tiles in total now. Setting the zoom to 80% shows the missing
> row but it also decreases the overall font size and like other persons
> already said here, it always reverts back to 100%, when you close the tab.
> In about:config 
> "browser.newtabpage.columns" is set to 5
> and
> "browser.newtabpage.rows" is set to 3.

Yes, I too tried tweaking settings in about:config but didn't work. Have been facing same issue.

Comment 10

[Moin Shaikh]

(In reply to neph from comment #5)
> I'm experiencing the same problem but for me the missing tiles don't even
> show for a split second, when opening a new tab, they simply don't get
> displayed at all.
> With FF 38 I had 3 rows and 4 columns of tiles but with FF 39 there are only
> 2 rows, so 8 tiles in total now. Setting the zoom to 80% shows the missing
> row but it also decreases the overall font size and like other persons
> already said here, it always reverts back to 100%, when you close the tab.
> In about:config 
> "browser.newtabpage.columns" is set to 5
> and
> "browser.newtabpage.rows" is set to 3.

Yes, I too tried tweaking settings in about:config but didn't work. Have been facing same issue. Changed default column values from 5 to 8 for browser.newtabpage.columns in about:config and restarted browser too, but still it showed default 5 columns.

Comment 11

[Loic]

Is it possible for someone affected by this bug to use the tool mozregression (see http://mozilla.github.io/mozregression/ for details)?

As FF38 worked, just run the line "mozregression --good-release 38".

Comment 12

[Moin Shaikh]

(In reply to Loic from comment #11)
> Is it possible for someone affected by this bug to use the tool
> mozregression (see http://mozilla.github.io/mozregression/ for details)?
> 
> As FF38 worked, just run the line "mozregression --good-release 38".

I will try to give it a go. Thanks for sharing this. Hopefully, I will use this tool and let you know the outcome

Comment 13

[Loic]

Maybe you'll need to populate the New Tab page if the bug doesn't surface, because each nighlty downloaded by Mozreg is started with a fresh profile. But you can run Mozreg with the profile of your choice, check the FAQ to do it.

Comment 14

[Loic]

(and I recommend to duplicate your current profile if you want to run Mozreg with it)

Comment 15

[Moin Shaikh]

(In reply to Loic from comment #14)
> (and I recommend to duplicate your current profile if you want to run Mozreg
> with it)

Thanks for suggestions Loic. Unfortunately, having hard time in installing Mozregression. Trying to fix it asap but it always get stuck while installing it. Will keep you posted.

Comment 18

[Loic]

No QA of this bug? It has been reported many times on SUMO and various community websites.

Comment 19

[Loic]

Reporter, could you use mozregression to find a regression range, please. (see comment #11)

Comment 22

[Alice0775 White]

Steps to reproduce
1. Make sure height of content area is 582px
2. Open New tab

Actual results:
 the second row of thumbnails appear and disappear in a moment


Regression window:
https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=400c2a449d8d&tochange=82ae3b4e2215

Suspect:
41d6e4de79be	Marina Samuel — Bug 1126188: Show suggested tile explanation text under a suggested tile. r=adw

Comment 23

[Loic]

Marina, can you check this bug, many users reported this issue when FF39 has been released.

Comment 24

[Marina Samuel [:emtwo]]

Hi Loic, this change was actually intentional - we reduced the number of visible rows in order to create room for messaging about suggested tiles beneath a tile. We're considering alternative solutions here, one which may be simply allowing the new tab page to be scroll-able.

Comment 25

[Oliver]

(In reply to Marina Samuel [:emtwo] from comment #24)
> Hi Loic, this change was actually intentional - we reduced the number of
> visible rows in order to create room for messaging about suggested tiles
> beneath a tile. We're considering alternative solutions here, one which may
> be simply allowing the new tab page to be scroll-able.

What an autocratic way of software development ...
In my opinion, you should either ask users first, or (better) make it configurable.
You have chosen the worst way - making one-way decisions for others and not care whether the users will like it or not.
Making it scrollable is the same bad path again.
Let the users decide whether they want messaging about suggested tiles or not - I bet most users will decline it.
My guess is that if you ask 100 users, at least 70% will say, the more tiles they see, the better. Because it is a nice way for visual bookmarks for most often sites.

The feedback should show clearly, that users want as many tiles visible as possible. And again no: making it scrollable would be a very cheap quick and dirty solution.

Thanks for reading.

Comment 26

[Loic]

(In reply to Marina Samuel [:emtwo] from comment #24)
> Hi Loic, this change was actually intentional - we reduced the number of
> visible rows in order to create room for messaging about suggested tiles
> beneath a tile. We're considering alternative solutions here, one which may
> be simply allowing the new tab page to be scroll-able.

The implementation is visually bad, check the screenshots in this bug.
The user sees a new tab page with the half of its tiles, like if some place was wasted instead of displaying more tiles, and he thinks it's a bug.

Comment 27

[Marina Samuel [:emtwo]]

(In reply to Loic from comment #26)
> (In reply to Marina Samuel [:emtwo] from comment #24)
> > Hi Loic, this change was actually intentional - we reduced the number of
> > visible rows in order to create room for messaging about suggested tiles
> > beneath a tile. We're considering alternative solutions here, one which may
> > be simply allowing the new tab page to be scroll-able.
> 
> The implementation is visually bad, check the screenshots in this bug.
> The user sees a new tab page with the half of its tiles, like if some place
> was wasted instead of displaying more tiles, and he thinks it's a bug.

I've looked at the screenshot...maybe we can center the tiles vertically so that it doesn't look like space is wasted?

Comment 28

[penguino]

(In reply to Marina Samuel [:emtwo] from comment #27)
> 
> I've looked at the screenshot...maybe we can center the tiles vertically so
> that it doesn't look like space is wasted?

I have another suggestion. If it ain't broke, don't fix it.

Comment 29

[neph]

(In reply to Marina Samuel [:emtwo] from comment #24)
> Hi Loic, this change was actually intentional - we reduced the number of
> visible rows in order to create room for messaging about suggested tiles
> beneath a tile. We're considering alternative solutions here, one which may
> be simply allowing the new tab page to be scroll-able.
Check out this the layout of my "new tab" page:
http://i.imgur.com/woVK636.png
With 16:9 it's even worse:
http://i.imgur.com/fiF0pNQ.png
There's enough space for at least another row, which I had with FF38 (and I liked it).
I have to agree with Oliver and Loic, the way it is now simply looks like space is wasted. And no, I don't want suggested tiles, that's why I pinned all of mine, so it wouldn't change them every time I visit a new website.
A second "no": Making it scrollable would be really cheap and since I only use the "new tab" page for my most visited sites, not for having half of my bookmarks there, it's simply unnecessary.
And a third "no": Centering the tiles vertically won't make it any better, there would still be way too much unused space.
Just change it back to how it was with FF38 and I believe that most users here would be perfectly fine with it.

(In reply to Oliver from comment #25)
> The feedback should show clearly, that users want as many tiles visible as
> possible.
Exactly, thank you.

Comment 30

[Carl Fletcher]

(In reply to penguino from comment #28)
> (In reply to Marina Samuel [:emtwo] from comment #27)
> > 
> > I've looked at the screenshot...maybe we can center the tiles vertically so
> > that it doesn't look like space is wasted?
> 
> I have another suggestion. If it ain't broke, don't fix it.

So true

Comment 31

[Lawrence Mandel [:lmandel] (use needinfo)]

Tracking 40+ as I'd like to determine if we're going to respond to this feedback. We are very late in the 40 cycle so this is in no way a promise about taking a change in 40.

Marina - This looks like a user visible regression in functionality even though it was an intentional change. What options do we have? Can we eliminate some of the whitespace at the top of the page to accommodate for this change? Is there space in the original layout that we can use for the suggested tiles message?

Comment 32

[Marina Samuel [:emtwo]]

The Content Services team met yesterday to discuss the action to take on this bug. Some possibilities we considered were:

1) Shrink tile sizes to fit more on-screen - this would not work well with pre-determined directory tile sizes and existing images/thumbnails may not scale well

2) Reduce the spacing between tiles for users who only want to see their top sites and add spacing for users who have suggestions - this might lead to bad or confusing UX when users switch back and forth between having and not having suggestions.

3) Make the page scrollable - There needs to be some bounds to the scrolling as to not overwhelm users with too many tiles at once. Aaron has mocked this idea here: https://www.dropbox.com/sh/6j1z0hy2148yar6/AAAW_9lJMgvsk9UPhRZlEmQHa/Presentations/Fx43_NT_Feeds_Groups_Feedback_V2.pdf (pages 31-39) However, implementing this scrolling ability in its entirety will take time and we want to deal with this problem in a timely manner.

And we concluded the next steps will be:

1) Move the search bar up to create more room (bug 1186584) - but this won't guarantee users another row of tiles.

2) We will implement a scrollable newtab as in Aaron's mocks (#3 above) but our first step is to add a simple scrolling mechanism with a minimum of at least 3 visible rows

Comment 33

[penguino]

Why can't you just go back to the way it was before?

Comment 34

[Sylvestre Ledru [:Sylvestre]]

Marina, do you have a time frame for this?
Anyway, this is too late for 40.

Comment 35

[neph]

(In reply to penguino from comment #33)
> Why can't you just go back to the way it was before?

I agree, why can't you simply change it back to how it was?
Which means: Make the border at the top and the bottom smaller (look at the screenshots I posted in comment 29, there's way too much unused space!) but don't change the spacing between the rows or the size of the tiles themselves. This way more rows can be displayed - with a maximmum of 3 rows and 5 columns or whatever the settings in about:config are set to - and everyone's happy. If there's someone, who doesn't like having 3 rows (which I doubt that there are many of), they can still use the about:config settings to change it but at least the other users can have it the way they had it before the "dreaded" version 39.
If you do want to make it scrollable, please let it be a config option because I sure as hell don't want it to display 5 or even 10 rows - that simply defeats the purpose of having a quick access to the most favorite websites.

Comment 36

[Marina Samuel [:emtwo]]

Sylvestre, I'll aim for a fix in Nightly before merge date (Aug 10)

Comment 37

[maxim zhilyaev]

Attachment: v1. making newtab scroolable

setting max-height to height makes grid scroolable in case the last row does not fit the screen.  tested in various configurations, the fix appears to achieve desired functionality.

Comment 38

[Marina Samuel [:emtwo]]

Comment on attachment 8644448 [details] [diff] [review]
v1.  making newtab scroolable

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

This looks good overall. Can we also add a margin-top for the search bar because it looks stuck to the top of the page.

Comment 39

[Marina Samuel [:emtwo]]

Comment on attachment 8644448 [details] [diff] [review]
v1.  making newtab scroolable

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

::: browser/base/content/newtab/grid.js
@@ +198,2 @@
>      this._node.style.height = this._computeHeight() + "px";
> +    this._node.style.maxHeight = this._node.style.height;

Btw, I believe you don't need this line at all since we have a fixed height now

Comment 40

[maxim zhilyaev]

(In reply to Marina Samuel [:emtwo] from comment #39)
> Comment on attachment 8644448 [details] [diff] [review]
> v1.  making newtab scroolable
> 
> Review of attachment 8644448 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: browser/base/content/newtab/grid.js
> @@ +198,2 @@
> >      this._node.style.height = this._computeHeight() + "px";
> > +    this._node.style.maxHeight = this._node.style.height;
> 
> Btw, I believe you don't need this line at all since we have a fixed height
> now

The line is actually necessary, for otherwise the tip of the next row of tiles is visible.

Comment 42

[maxim zhilyaev]

Attachment: v2. fixed margin-top per reviewer comment

Comment 43

[maxim zhilyaev]

Attachment: v3. searhc-bar margins

Comment 44

[Pulsebot]

https://hg.mozilla.org/integration/fx-team/rev/6f99042dd397

Comment 45

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/6f99042dd397

Comment 46

[Moltres Rider]

I got news for you!!! it STILL DOES it on my latest version of Firefox!!!

Comment 47

[Sylvestre Ledru [:Sylvestre]]

Please don't reopen bugs. We know that is not fixed in 40 (I guess it is the version you are referring too). As you can see in the "Tracking Flags:" section, we decided not to fix it for 40. An uplift is coming to see this bug fixed in 41 & 42 (it is fixed in nightly aka 43).

Comment 48

[Ed Lee :Mardak]

(In reply to maxim zhilyaev from comment #40)
> (In reply to Marina Samuel [:emtwo] from comment #39)
> > >      this._node.style.height = this._computeHeight() + "px";
> > > +    this._node.style.maxHeight = this._node.style.height;
> > Btw, I believe you don't need this line at all since we have a fixed height
> > now
> The line is actually necessary, for otherwise the tip of the next row of
> tiles is visible.
Can you take a screenshot of that? You shouldn't need maxHeight if the goal is to always show 3 rows of tiles by default. If it is showing some extra, it's probably because computeHeight() needs to be updated.

Comment 49

[maxim zhilyaev]

Attachment: ScreenShot shows appearance of top of the next tile rows without maxHeight

Comment 50

[maxim zhilyaev]

Attachment: Top of the next tile rows disappears of maxHeight=height

Comment 51

[maxim zhilyaev]

I am having serious issues with passing mochitests on aurora and beta with the patch applied.
Will take extra efforts to uplift

Comment 52

[maxim zhilyaev]

Attachment: v1. Aurora Patch

Approval Request Comment
[Feature/regressing bug #]: 1180387
Also note Bug 1195699 for porting relevant test fixes to ngithly
[User impact if declined]: 
Significant.  Numerous users are upset about loosing 3d row of history tiles. 
[Describe test coverage new/current, TreeHerder]:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=13d9488cfe65
[Risks and why]: 
[String/UUID change made/needed]: None

Comment 53

[maxim zhilyaev]

Attachment: v1. Beta Patch

Approval Request Comment
[Feature/regressing bug #]:1180387
Also note bug 1195699 to port test changes to nightly
[User impact if declined]:
Significant. Numerous users' concern about loosing 3d row of history tiles 
[Describe test coverage new/current, TreeHerder]:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=cdbf72dc8503
[Risks and why]:
Mild change is newtab behavior 
[String/UUID change made/needed]: None

Comment 54

[Sylvestre Ledru [:Sylvestre]]

(In reply to maxim zhilyaev from comment #53)
> [Risks and why]:
> Mild change is newtab behavior 
Even a mild change can add risks to the release. Could you be a bit more explicit on this? Thanks

Comment 55

[maxim zhilyaev]

(In reply to Sylvestre Ledru [:sylvestre] from comment #54)
> (In reply to maxim zhilyaev from comment #53)
> > [Risks and why]:
> > Mild change is newtab behavior 
> Even a mild change can add risks to the release. Could you be a bit more
> explicit on this? Thanks

My risk assertion is based on passing automatic tests and limited amount of manual testing. Numerous tests failed when patch was applied to aurora/beta, but the failure causes were identified and fixed. Since I spent considerable efforts making patch work on aurora/beta, I do feel more comfortable about its quality.  

As far as I can tell, aurora patch passes try-tests cleanly, and beta patch try-run sees some failures which I believe are unrelated to the changes made.   Neither I found problems by manual testing each build.  This makes me believe that risks are moderate. 

Which other tests (in addition to try runs) would you recommend I run to reduce the risk exposure?

Comment 56

[Ed Lee :Mardak]

Bug 1194895 has been filed as a regression of this bug, so I would assume it should be fixed before this gets landed or there should be an explanation of why this should land without bug 1194895 fixed.

Comment 57

[maxim zhilyaev]

(In reply to Ed Lee :Mardak from comment #56)
> Bug 1194895 has been filed as a regression of this bug, so I would assume it
> should be fixed before this gets landed or there should be an explanation of
> why this should land without bug 1194895 fixed

I am, frankly, torn on that. We probably should attempt to land 1194895, but then there's no consensus on how 1194895 should be fixed. I have attached a patch to 1194895 that fixes the worst-case scenario, but that did not meet dholbert's criteria for being non-regression.

My concern is that by requiring 1194895 fix, we may miss beta uplift time-window, and make already complaining users even more irritated about their 3-d row of tiles not visible.  I believe this is a far worse evil that not landing 1194895.

I would recommend the following sequence of steps:
-  Uplift this patch to ensure that immediate users' complains are resolved regardless.
-  Attempt to uplift 1194895 patch for not showing empty rows of tiles on empty history.
-  Put the remaining issues with 1194895 into a separate bug as non-regression, so we can time to undress them properly

Comment 58

[maxim zhilyaev]

(In reply to maxim zhilyaev from comment #57)
>> so we can time to undress them properly

I meant - so we have time to address them properly

Comment 59

[maxim zhilyaev]

NOTE to release drivers:

Team decided to go with the fix to 1194895.
Fix to 1194895 is a refactor of 1180387 changes.
We will request uplift to aurora/beta for 1194895.
I am cancelling uplift requests in this bug.

Comment 60

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Given comment 59, untracking this bug and adding a tracking flag to bug 1194895.

Comment 63

[Mohammad Maruf Islam]

I have reproduced this bug with Firefox 39.0 (Build: 20150630154324)on 
windows 8.1 pro x64 with the instructions from comment 0 .

Verified as fixed with Latest Firefox Nightly 44.0a1 (Build ID: 20151001030236)
Mozilla/5.0 (Windows NT 6.3; WOW64; rv:44.0) Gecko/20100101 Firefox/44.0

Verified as fixed with Latest Firefox beta 42.0b3 (Build ID: 20151001142456)
Mozilla/5.0 (Windows NT 6.3; WOW64; rv:42.0) Gecko/20100101 Firefox/42.0

Comment 64

[neph]

I just updated to FF42 and finally my third row is back. Thank you!
But... Why is there a scrollbar now? There's a 1cm gap between the rows and it completely shows the 3rd row (plus 5mm of the white background) but it still lets me scroll for another 5mm to see more of the whiteness.
http://i.imgur.com/KXIC6U6.png
Also: When I make the window smaller (only height-wise), it now only shows 3 tiles per row instead of the usual 4, plus there's still that weird scrollbar to show me 5 more mm of the background. Why?!

Comment 65

[Andre Klapper]

@neph: Please ask support questions in a support forum as they are out of scope for the topic of this very bug report. Thanks!

Comment 66

[Nazir Ahmed Sabbir [:NaSb]]

Reproduced this bug in Nightly 42.0a1 (2015-07-04) ; (Build ID : 20150704030210) on Linux, 64 Bit  

This Bug is now verified as fixed on Latest Firefox Beta 43.0b3

Build ID : 20151112144305
User Agent : Mozilla/5.0 (X11; Linux x86_64; rv:43.0) Gecko/20100101 Firefox/43.0

As it is also verified on Windows (Comment 63), Marking it as verified!