1199795 - New Tab's height doesn't update the same moment when I delete extra tiles/choose "show blank page"

Status: RESOLVED FIXED

(Firefox :: New Tab Page, defect)

Description

[arni2033]

Attachment: Video - scenario (B) - New Tab's height doesn't update the same moment when I delete extra tiles or choose 'show blank page'.webm

STR:   (Win7_64bit, Nightly 43.0a1 32bit   ID 20150828030205, new profile)

[There's 2 scenarios: (A) and (B). Watch video to see scenario (B) ]

0.   Open New Tab, click Gear button, uncheck "Include suggested sites". Adjust the window's height so that only 4 tiles in a row were visible, and only 2 or 1 rows were fully visible.
1. Fill all visible tiles on New Tab +1 tile. If you already have enough sites there to cause
   overflow, then skip this step.
  1.1. Open attached page, click textarea, press Ctrl+A to select all text, press Ctrl+C to copy
  1.2. Press Ctrl+B to open bookmarks, click "Bookmarks Menu", press Ctrl+V to create bookmarks
  1.3. Right-click "Bookmarks Menu", click "Open all in tabs" menuitem.
2.   Open New Tab, make sure that there's overflow on New Tab (vertical scrollbar is visible)
3.A) Click Gear button, select "Show blank page"
3.B) Delete extra tiles, so that the existing tabs were fully visible (that means that
     there should be no overflow according to bug 1194895)
4.   Click "New tab" button/press Ctrl+T to open new tab
5.   Repeat Step 4.

Result:       After Step 4 scrollbar is still visible. It disappears only after Step 5.
              If I set browser.newtab.preload -> true then scrollbar disappears after Step 4.
Expectations: Scrollbar should disappear after Step 3.

Comment 1

[arni2033]

Attachment: Page with URLs to create history - New Tab's height doesn't update the same moment when I delete extra tiles or choose 'show blank page'.html

Comment 2

[maxim zhilyaev]

Attachment: v1. Make gGrid.refresh() also resize

A minor change in grid refresh logic.  Olivier and Tim to validate that it's OK to land in nightly.  Changes are minor and should be easy to port to remove newtab.

Comment 3

[Olivier Yiptong [:oyiptong]]

Comment on attachment 8660939 [details]
v1.  Make gGrid.refresh() also resize

>   /**
>-   * Renders the grid, including cells and sites.
>+   * Renders and resizes the gird. _resizeGrid() call is needed to ensure
"Renders and resizes the grid."

>+   * that scrollbar disappears when the bottom row becomes empty following
>+   * the block action, or tile display is turmed off via cog menu
>    */
>+
>   refresh() {
>+    this._refresh();
is this._refresh defined anywhere?

Comment 4

[Tim Spurway [:tspurway]]

Max, as you know, I would like to see all dev on the newtab page frozen, and any patches applied to the remote new tab page.  The exceptions should be rare and serious.  I don't have a good feeling for how serious this bug is.  oyiptong?

Comment 5

[Olivier Yiptong [:oyiptong]]

Doesn't look that serious, in that it is a cosmetic issue.

We certainly want to fix this bug, however as you say, it is a bug-fix to a feature that has been introduced when the newtab page should've been frozen.

Max: will it fix all scrollbar-related issues? I don't think that bug is acceptable for good UX.

I propose two solutions: we back out the parent change: bug 1194895 and implement scrollbars in the remote new tab page, or we fix this bug for good and port it over to the remote new tab page.

The status quo is a broken window that irks me.

Comment 6

[Olivier Yiptong [:oyiptong]]

also: once you rename refresh to _refreshGrid, that makes the new refresh function dead code, doesn't it?

Comment 7

[Olivier Yiptong [:oyiptong]]

ah pardon me: it is used in page.js

Comment 8

[maxim zhilyaev]

Attachment: v2. fixed typo

Comment 9

[Marcos Caceres [:marcosc]]

Comment on attachment 8661969 [details] [diff] [review]
v2. fixed typo

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

Looks pretty straight forward.

Comment 10

[Pulsebot]

https://hg.mozilla.org/integration/fx-team/rev/535c11ecde00

Comment 11

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/535c11ecde00