Closed Bug 1180387 Opened 5 years ago Closed 5 years ago

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

Categories

(Firefox :: New Tab Page, defect)

39 Branch
defect
Not set
Points:
8

Tracking

()

RESOLVED FIXED
Firefox 43
Iteration:
43.1 - Aug 24
Tracking Status
firefox39 - wontfix
firefox40 + wontfix
firefox41 - wontfix
firefox42 - affected
firefox43 --- verified

People

(Reporter: moltres.facesits.justin.coolidge, Assigned: mzhilyaev)

References

(Blocks 2 open bugs)

Details

(Keywords: regression, Whiteboard: [testday-20151002])

Attachments

(6 files, 2 obsolete files)

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
This is also reported by multiple people on the german camp-firefox forum.
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.
(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!
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.
[Tracking Requested - why for this release]:

Bug reported on the French community board too:
https://forums.mozfr.org/viewtopic.php?f=5&t=125113
Status: UNCONFIRMED → NEW
Ever confirmed: true
Keywords: regression
Summary: new tab page doesn't show websites → New Tab Page doesn't show websites (wrong number of rows or lines)
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
'browser.newtab.preload' is not the problem.
(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.
(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.
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".
(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
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.
(and I recommend to duplicate your current profile if you want to run Mozreg with it)
(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.
Duplicate of this bug: 1181774
Duplicate of this bug: 1181720
No QA of this bug? It has been reported many times on SUMO and various community websites.
Reporter, could you use mozregression to find a regression range, please. (see comment #11)
Flags: needinfo?(moltres.facesits.justin.coolidge)
Duplicate of this bug: 1184138
Duplicate of this bug: 1184437
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
Marina, can you check this bug, many users reported this issue when FF39 has been released.
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.
Flags: needinfo?(msamuel)
(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.
(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.
(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?
(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.
(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.
(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
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?
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
Why can't you just go back to the way it was before?
Marina, do you have a time frame for this?
Anyway, this is too late for 40.
Flags: needinfo?(msamuel)
(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.
Sylvestre, I'll aim for a fix in Nightly before merge date (Aug 10)
Flags: needinfo?(msamuel)
Assignee: nobody → msamuel
Assignee: msamuel → mzhilyaev
Attached patch v1. making newtab scroolable (obsolete) — Splinter Review
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.
Attachment #8644448 - Flags: review?(msamuel)
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.
Attachment #8644448 - Flags: review?(msamuel) → review+
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
(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.
Blocks: 1176364
No longer blocks: 1126188
Depends on: 1186567
Depends on: 1186584
Blocks: 1172713
No longer blocks: 1176364
Duplicate of this bug: 1193466
Attachment #8644448 - Attachment is obsolete: true
Attachment #8647114 - Attachment is obsolete: true
https://hg.mozilla.org/mozilla-central/rev/6f99042dd397
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 43
I got news for you!!! it STILL DOES it on my latest version of Firefox!!!
Status: RESOLVED → REOPENED
Flags: needinfo?(moltres.facesits.justin.coolidge)
Resolution: FIXED → ---
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).
Status: REOPENED → RESOLVED
Closed: 5 years ago5 years ago
Resolution: --- → FIXED
(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.
Blocks: Sprint_CS_S1
I am having serious issues with passing mochitests on aurora and beta with the patch applied.
Will take extra efforts to uplift
Depends on: 1195699
Attached patch v1. Aurora PatchSplinter Review
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
Attachment #8649231 - Flags: approval-mozilla-aurora?
Attached patch v1. Beta PatchSplinter Review
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
Attachment #8649232 - Flags: approval-mozilla-beta?
(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
Flags: needinfo?(mzhilyaev)
(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?
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.
(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
Flags: needinfo?(mzhilyaev)
(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
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.
Attachment #8649231 - Flags: approval-mozilla-aurora?
Attachment #8649232 - Flags: approval-mozilla-beta?
Depends on: 1195321
No longer depends on: 1195321
Depends on: 1195321
Given comment 59, untracking this bug and adding a tracking flag to bug 1194895.
Iteration: --- → 43.1 - Aug 24
Points: --- → 8
Depends on: 1202206
Depends on: 1199795
Depends on: 1199801
Depends on: 1199933
Depends on: 1202251
Depends on: 1202253
Duplicate of this bug: 1202206
Duplicate of this bug: 1186567
Blocks: 1205313
No longer blocks: 1205313
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
Whiteboard: [testday-20151002]
Depends on: 1210091
QA Whiteboard: [good first verify]
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?!
@neph: Please ask support questions in a support forum as they are out of scope for the topic of this very bug report. Thanks!
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!
QA Whiteboard: [good first verify] → [good first verify][testday-20151113]
You need to log in before you can comment on or make changes to this bug.