1337858 - Replace CopyFavicon in Docshell with a proper API

Status: RESOLVED FIXED

(Toolkit :: Places, defect, P1)

Description

[Marco Bonardo [:mak] (Away Apr 25 - May 5)]

The current one will only copy one payload, that is not what we want in general.

Comment 1

[Marco Bonardo [:mak] (Away Apr 25 - May 5)]

while here could also fix back/forward menu to use page-icon instead of getFaviconUrlForPage

Comment 2

[Marco Bonardo [:mak] (Away Apr 25 - May 5)]

Attachment: Bug 1337858 - Replace CopyFavicon in the Docshell with a proper favicons API.

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

Comment 3

[Mark Banner (:standard8)]

Comment on attachment 8870735 [details]
Bug 1337858 - Replace CopyFavicon in the Docshell with a proper favicons API.

https://reviewboard.mozilla.org/r/142224/#review149212

Looks good, r=Standard8

Comment 4

[Marco Bonardo [:mak] (Away Apr 25 - May 5)]

Comment on attachment 8870735 [details]
Bug 1337858 - Replace CopyFavicon in the Docshell with a proper favicons API.

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

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/d48d0c5b0191
Replace CopyFavicon in the Docshell with a proper favicons API. r=standard8

Comment 6

[Ryan VanderMeulen [:RyanVM]]

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

Comment 7

[Gerry Chang [:gchang]]

54 RC build is released. Mark 54 won't fix.

Comment 8

[Marco Bonardo [:mak] (Away Apr 25 - May 5)]

it's actually unaffected, this work is just needed for the new favicons store.