Chris Lee [:clee] Reporter
	Description • 13 years ago

Overall functionality of the New Tab speed dial feature has been implemented.

There are two changes we'd recommend:

1) There appears to be some slight animation of the thumbnails appearing when a new tab is spawned.  From a responsiveness perspective, if we could remove the animation (assuming it was intentional), that will help users' perception (new and existing) of any slowness in the Firefox browser.

2) For new users with fresh profile, there isn't any content to populate the New Tab thumbnails so the first run experience may seem a bit strange.  

The suggestion here is to pre-populate the first 2-3 thumbnails (L to R):

*Getting Started (http://www.mozilla.org/en-US/firefox/central/)
*First Time with Add-ons (https://addons.mozilla.org/en-US/firefox/)
    **Recommend we change to the App Marketplace once that launches
*About Us (http://www.mozilla.org/en-US/about/) (optional, UX to weigh in)

As users generate a browser history, these thumbnails should quickly be replaced with their top visited sites.

	Tim Taubert [:ttaubert] (inactive)
	Comment 1 • 13 years ago

(In reply to Chris Lee [:clee] from comment #0)
> 1) There appears to be some slight animation of the thumbnails appearing
> when a new tab is spawned.  From a responsiveness perspective, if we could
> remove the animation (assuming it was intentional), that will help users'
> perception (new and existing) of any slowness in the Firefox browser.

I fixed this in the latest push to the UX branch and it's included in tomorrow's UX Nightly. I'd say this bug should by about [2] which I think is a very good suggestion.

	Chris Lee [:clee] Reporter
	Comment 2 • 13 years ago

(In reply to Tim Taubert [:ttaubert] from comment #1)
> (In reply to Chris Lee [:clee] from comment #0)
> > 1) There appears to be some slight animation of the thumbnails appearing
> > when a new tab is spawned.  From a responsiveness perspective, if we could
> > remove the animation (assuming it was intentional), that will help users'
> > perception (new and existing) of any slowness in the Firefox browser.
> 
> I fixed this in the latest push to the UX branch and it's included in
> tomorrow's UX Nightly. I'd say this bug should by about [2] which I think is
> a very good suggestion.

[1] I'm on the 11/28 UX build and I'm still observing some slight lag when spawning a new tab.  Are you seeing this as well?  Compared to Chrome the lag is definitely noticeable.  

Tried on both OS X 10.7 and Win7.

[2] Assuming [1] is fixed and I'm just not seeing it, I agree we should make this bug focused on [2].

	Jennifer Morrow [:Boriss] (UX)
	Comment 3 • 13 years ago

(In reply to Chris Lee [:clee] from comment #0)
> Overall functionality of the New Tab speed dial feature has been implemented.
> 
> There are two changes we'd recommend:
> 
> 1) There appears to be some slight animation of the thumbnails appearing
> when a new tab is spawned.  From a responsiveness perspective, if we could
> remove the animation (assuming it was intentional), that will help users'
> perception (new and existing) of any slowness in the Firefox browser.

I don't think the animation itself is the problem, but rather that on current nightly builds it's not running very smoothly.  Especially with many tabs open, I'm seeing the thumbnails fade into position bit by bit in three or four distinct stages rather than smoothly.  If we can make this animation smooth, the responsiveness will feel buch better.  Simply removing the animation would mean that we flickr from a bright white new tab page to a populated one in one go, which I really don't recommend for its flashing effect.

	Tim Taubert [:ttaubert] (inactive)
	Comment 4 • 13 years ago

(In reply to Chris Lee [:clee] from comment #2)
> [1] I'm on the 11/28 UX build and I'm still observing some slight lag when
> spawning a new tab.  Are you seeing this as well?  Compared to Chrome the
> lag is definitely noticeable.

There is a small lag that is the same as when opening about:home for example in a new tab. I removed the animation so all we see is the time it takes to render the page. I think we won't get lower than that :/

Fwiw, Chrome feels the same to me (sometimes even slower) - at least on my MacBook Air...

(In reply to Jennifer Morrow [:Boriss] (Firefox UX) from comment #3)
> I don't think the animation itself is the problem, but rather that on
> current nightly builds it's not running very smoothly.  Especially with many
> tabs open, I'm seeing the thumbnails fade into position bit by bit in three
> or four distinct stages rather than smoothly.  If we can make this animation
> smooth, the responsiveness will feel buch better.

The 'problem' is, to animate slides and fades I use our moz-transition CSS properties. I can't speed those up as they are a core implementation and should be quick. If they're not we should make them faster because every web site using that would benefit (though that surely isn't that simple). I'm not sure if that has to do with the CC pauses that we work on with the generational GC?! Maybe that would mitigate the problem.

> Simply removing the
> animation would mean that we flickr from a bright white new tab page to a
> populated one in one go, which I really don't recommend for its flashing
> effect.

Should I re-add the effect? If so, maybe not with 200ms duration but 50-100ms?

	Tim Taubert [:ttaubert] (inactive)
	Comment 5 • 13 years ago

(In reply to Tim Taubert [:ttaubert] from comment #4)
> The 'problem' is, to animate slides and fades I use our moz-transition CSS
> properties. I can't speed those up as they are a core implementation and
> should be quick. If they're not we should make them faster because every web
> site using that would benefit (though that surely isn't that simple). I'm
> not sure if that has to do with the CC pauses that we work on with the
> generational GC?! Maybe that would mitigate the problem.

Btw, Panorama also uses CSS transitions for their animations and suffers from the same problems (though I think there are some optimizations applicable in some places).

	Chris Lee [:clee] Reporter
	Comment 6 • 13 years ago

> There is a small lag that is the same as when opening about:home for example
> in a new tab. I removed the animation so all we see is the time it takes to
> render the page. I think we won't get lower than that :/

I'm good with that after playing with it a bit more.

> > Simply removing the
> > animation would mean that we flickr from a bright white new tab page to a
> > populated one in one go, which I really don't recommend for its flashing
> > effect.
> 
> Should I re-add the effect? If so, maybe not with 200ms duration but
> 50-100ms?

Agree that we don't want the 'flashing effect'.  Adding it back in with a 50-100ms seems to make sense.

	Andres Hernandez [:andreshm]
	Comment 8 • 12 years ago

I have the following questions and comments regarding this bug:

1. In order to pre populate the links, we need to detect the first time a profile runs. I create a new pref, since I can't not use any of the existing preferences, like 'browser.startup.homepage_override.mstone', because the new tab module runs until a new tab is opened and in that moment those preferences are already changed.

2. By default, the first run page is loaded and added to the browser history. So, I'm using only two links to pre populate the new tab: http://www.mozilla.org/firefox/central/ and https://addons.mozilla.org/firefox/.

3. When building the link object, we need the url and the title of the page. Currently, I'm not using any title, so the url appears as the title. Should we have the titles localized for this?

4. With the pre populated links, the page thumbnails are empty, since those links have not been loaded yet. Any suggestions for this?

	Tim Taubert [:ttaubert] (inactive)
	Comment 9 • 12 years ago

Comment on attachment 615740 [details] [diff] [review]
First approach

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

Regarding the general approach I'd say we switch to pinning these sites at the first three positions. Users might want to remove them after they got used to the feature and so they might learn about that. Also, it's quite hard to show these sites on the first run as they actually would need to be inserted with the right frecency points and then decide when to expire them again.

We should call PinnedLinks.pin(link, index) for each site when NewTabUtils.jsm gets loaded, 'browser.newtabpage.firstrun' is false and Storage.isEmpty().

I think we should ask the UX/visual team for some nice thumbnails that we ship for those sites. The thumbnails would need to be decently sized and shouldn't contain any text because that would need to be localized. Maybe some type of icons will do. They would of course be overridden once the user actually visits those pages.

I'm not quite sure how to handle page titles. We could put them in a DTD file and name them slightly different (just because we can), the UX team should be able to help with that. Titles would then be translated by l10n folks.

::: browser/modules/NewTabUtils.jsm
@@ +31,5 @@
>  
> +// The preload list of links.
> +const PRELOAD_LINKS = [
> +  "http://www.mozilla.org/firefox/central/",
> +  "https://addons.mozilla.org/firefox/"

We should include http://www.mozilla.org/about/ unless UX says otherwise.

	Andres Hernandez [:andreshm]
	Comment 10 • 12 years ago

With this update, the 3 links with titles are pinned and loaded fine. But, is still pending to add the default thumbnails for the 3 links. 

We need the images from UX, but in which folder should I place those images? 

I also played around with PageThumbs module, since in order to add the default images we need to add a method that creates or copies those images to the respective folder in the thumbnails directory. We can't not use the existing capture method, but we need the PageThumbsStorage method that writes the file. Is there a way to get the stream of the image without using a canvas?

	Tim Taubert [:ttaubert] (inactive)
	Comment 11 • 12 years ago

(In reply to Andres Hernandez [:andreshm] from comment #10)
> We need the images from UX, but in which folder should I place those images? 

We should put these images into browser/themes/*stripe/newtab/.

shorlander, boriss, what do you think of this approach? UX needs to decide about the pages we want to show and we would need some nice thumbnails to have as the default.

	Tim Taubert [:ttaubert] (inactive)
	Comment 12 • 12 years ago

(In reply to Andres Hernandez [:andreshm] from comment #10)
> I also played around with PageThumbs module, since in order to add the
> default images we need to add a method that creates or copies those images
> to the respective folder in the thumbnails directory.

You can use PageThumbsStorage.getFileForURL() to determine a thumbnails file name and create copies of the default thumbnails.

	Tim Taubert [:ttaubert] (inactive)
	Comment 13 • 12 years ago

Comment on attachment 633697 [details] [diff] [review]
Updated patch

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

Thank you, this looks good so far.

We should make this bug depend on bug 754671. It will introduce a NewTabUtils.init() method that is called when the browser starts. Also, this will start to implement an asynchronous thumbnail storage that we can use to asynchronously copy the default thumbnails.

I'd really like if all this code could be a self-contained object like this:

> let FirstRun = {
>   preloadLinks: [ ... ],
>   init: function () { /* called by NTU.init(), checks and sets pref */ },
>   preload: function () { /* applies first-run links */ }
> };

::: browser/modules/NewTabUtils.jsm
@@ +29,3 @@
>  // The preference that tells whether this feature is enabled.
>  const PREF_NEWTAB_ENABLED = "browser.newtabpage.enabled";
> +// The preference that tells whether is first run.

*whether it's the first run.

@@ +176,5 @@
> +   * Gets the number of items.
> +   */
> +  get length() {
> +    return this._data.count;
> +  },

We don't need this anymore when bug 722990 lands, yay!

@@ +697,5 @@
>    pinnedLinks: PinnedLinks,
>    blockedLinks: BlockedLinks
>  };
> +
> +(function() { this.init(); }).apply(NewTabUtils);

Will be called automatically with bug 754671.

	Ben Bucksch (:BenB)
	Comment 14 • 12 years ago

Animation:

(In reply to Chris Lee [:clee] from comment #0)
> 1) There appears to be some slight animation of the thumbnails appearing
> when a new tab is spawned.  From a responsiveness perspective, if we could
> remove the animation (assuming it was intentional), that will help users'
> perception (new and existing) of any slowness in the Firefox browser.

Agreed. The mere existence of the animation makes the page feel sluggish, because it should be there instantly, not feel like it's being "loaded".

clee, could you file a separate bug to remove the animation? There's bug 752839, but it's about fixing some bug about it, while we should be removing it entirely.

The animation runs very fast for me, in a split-second (200ms?), but still noticeable. I thought it wasn't even intentional, but genuine load slowness.

	Andres Hernandez [:andreshm]
	Comment 15 • 12 years ago

Updated patch, but is not final, since still need to add the default thumbnails.

	Tim Taubert [:ttaubert] (inactive)
	Comment 16 • 12 years ago

Comment on attachment 635117 [details] [diff] [review]
Updated patch

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

Looks good to me! We still need to wait for bug 754671 and feedback from UX about the thumbnails and links. Also we then need a method that asynchronously copies the thumbnails to their final locations.

::: browser/modules/NewTabUtils.jsm
@@ +623,5 @@
> +      this._preloadLinks();
> +    }
> +  },
> +
> +  _preloadLinks: function FirstRun_preload() {

_preloadLinks: function FirstRun_preloadLinks() {

	Tim Taubert [:ttaubert] (inactive)
	Comment 17 • 11 years ago

I think we should pick this up again. First, we should take Andres' patch and make it work with m-c tip. Once we have that we can ask the UX team about links we should use. Good thing is we don't have to worry about thumbnails anymore as bug 927688 tells that we will just generate those thumbnails on first run starting with Firefox 27.

	Kushagra Singh [:kushagra]
	Comment 18 • 11 years ago

hi tim. i would like to pick up on this bug and work on it. could you please help me with it?

	Ben Bucksch (:BenB)
	Comment 19 • 11 years ago

He wrote in comment 17:
> First, we should take Andres' patch and make it work with m-c tip.

	Matthew N. [:MattN]
	Comment 20 • 10 years ago

Mass-move to Firefox::New Tab Page.

Filter on new-tab-page-component.

	Annonomus Penguin
	Comment 21 • 10 years ago

#2 seems to be fixed on the latest nightly. Should this be marked completed?