1396104 - Use rich icons when getting top sites

Status: RESOLVED FIXED

(Firefox :: New Tab Page, enhancement)

Description

[Ed Lee :Mardak]

Ursula says 1 line change:

This is done via newtabutils's _addFavicons function - passing in a 0 to here: https://dxr.mozilla.org/mozilla-central/source/toolkit/modules/NewTabUtils.jsm?q=file%3Anewtabutils.jsm&redirect_type=single#877 to pull out largest favicon available

Comment 1

[Ursula Sarracini (:ursula)]

Attachment: Bug 1396104 - Use rich icons when getting top sites

Review commit: https://reviewboard.mozilla.org/r/177808/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/177808/

Comment 2

[Ed Lee :Mardak]

Comment on attachment 8906070 [details]
Bug 1396104 - Use rich icons when getting top sites

https://reviewboard.mozilla.org/r/177808/#review182876

::: toolkit/modules/NewTabUtils.jsm:961
(Diff revision 1)
>                  link.faviconLength = len;
> +                link.faviconSize = size;
>                }
>                return resolve(link);
>              });
> -        }).catch(() => {
> +        }, 0).catch(() => {

The indentation is all funky here. Your 0 here is the 2nd argument to `new Promise()` (notice the `.catch`)

Does this mean we don't actually need to pass in 0 to `getFaviconDataForPage`?

Comment 3

[Ed Lee :Mardak]

For styling, I could see the 0 on its own line with a comment, e.g.,

new Promise(resolve => {
  return Places.getFavicon(
    uriStuff,
    callbackStuff => {
    },
    0); // preferredWidth: get the biggest width available
}).catch

Where you only touch the line of the callback closing brace (add comma) and the 0 line.

Comment 4

[Ursula Sarracini (:ursula)]

Comment on attachment 8906070 [details]
Bug 1396104 - Use rich icons when getting top sites

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/177808/diff/1-2/

Comment 5

[Ed Lee :Mardak]

Comment on attachment 8906070 [details]
Bug 1396104 - Use rich icons when getting top sites

https://reviewboard.mozilla.org/r/177808/#review182888

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/d923714255be
Use rich icons when getting top sites r=Mardak

Comment 7

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/d923714255be