1421306 - Show rich icons for top sites that are in a tippy top service manifest

Status: RESOLVED FIXED

(Firefox :: New Tab Page, defect)

Description

[Ed Lee :Mardak]

Some top sites don't advertise the rich icons to firefox, but we can scrape them and make them available as a manifest with ios user agent to find more apple-touch icons. Users should see fewer screenshots avoiding issues like bug 1412505.

Fixed as part of export bug 1415812.
https://hg.mozilla.org/mozilla-central/rev/b4d1db669c93

Comment 1

[Ed Lee :Mardak]

gchang says this bug has missed 58.0b8. It doesn't have a patch with beta approval flag. Please attach the appropriate patch without install.rdf.in changes and request approval ASAP.

Comment 2

[Andrei Oprea [:andreio]]

Attachment: Bug 1421306 - Show rich icons for top sites that are in a tippy top service manifest.

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

Comment 3

[Andrei Oprea [:andreio]]

Comment on attachment 8933417 [details]
Bug 1421306 - Show rich icons for top sites that are in a tippy top service manifest.

Approval Request Comment
[Feature/Bug causing the regression]: feature https://bugzilla.mozilla.org/show_bug.cgi?id=1386767
[User impact if declined]: user experience on activity stream page is affected by lower quality topsite images
[Is this code covered by automated tests?]: yes
[Has the fix been verified in Nightly?]: yes
[Needs manual test from QE? If yes, steps to reproduce]: no
[List of other uplifts needed for the feature/fix]:
[Is the change risky?]: no
[Why is the change risky/not risky?]: covered by tests, already tested in beta
[String changes made/needed]:no

Comment 4

[Ed Lee :Mardak]

Comment on attachment 8933417 [details]
Bug 1421306 - Show rich icons for top sites that are in a tippy top service manifest.

https://reviewboard.mozilla.org/r/204342/#review209896

This is missing NewTabUtils changes

::: browser/extensions/activity-stream/data/content/activity-stream.bundle.js:1692
(Diff revision 1)
>   * @param acc Accumulator for reducer.
>   * @param topsite Entry in TopSites.
>   */
>  function countTopSitesIconsTypes(topSites) {
>    const countTopSitesTypes = (acc, link) => {
> -    if (link.tippyTopIcon) {
> +    if (link.tippyTopIcon || link.faviconRef === "tippytop") {

This requires NewTabUtils changes adding faviconRef.

Comment 5

[Andrei Oprea [:andreio]]

Comment on attachment 8933417 [details]
Bug 1421306 - Show rich icons for top sites that are in a tippy top service manifest.

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

Comment 6

[Ed Lee :Mardak]

Comment on attachment 8933417 [details]
Bug 1421306 - Show rich icons for top sites that are in a tippy top service manifest.

https://reviewboard.mozilla.org/r/204342/#review210244

Comment 7

[Gerry Chang [:gchang]]

Comment on attachment 8933417 [details]
Bug 1421306 - Show rich icons for top sites that are in a tippy top service manifest.

Fix a lower quality topsite images on on activity stream page. Beta58+.

Comment 8

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/6ff9a23d3212