834915 - audit elm usage of deprecated Places APIs

Status: RESOLVED FIXED

(Firefox for Metro Graveyard :: General, defect)

Description

[Marco Bonardo [:mak]]

Please check bug 834457 for a list of APIs that are going away, since some of the code went from mobile/xul it may contain some of them.

Comment 1

[Marco Bonardo [:mak]]

I looked at browser/metro, Brian is there any other location I should look at?

I found:
http://mxr.mozilla.org/projects-central/source/elm/browser/metro/base/content/bindings/browser.xml#132
http://mxr.mozilla.org/projects-central/source/elm/browser/metro/base/content/pages/aboutCertError.xhtml#28

these are easy to replace, I suggest using PlacesUtils.favicons.setAndFetchFaviconForPage

http://mxr.mozilla.org/projects-central/source/elm/browser/metro/base/content/bookmarks.js#211
http://mxr.mozilla.org/projects-central/source/elm/browser/metro/base/content/bookmarks.js#254

These are a bit more complicated since getFaviconImageForPage is synchronous, the replacement is PlacesUtils.favicons.getFaviconURLForPage, that is asynchronous and accepts a callback (http://mxr.mozilla.org/mozilla-central/source/toolkit/components/places/mozIAsyncFavicons.idl#149)

Comment 2

[Brian R. Bondy [:bbondy]]

Yes also in widget/windows/winrt please

Comment 3

[Marco Bonardo [:mak]]

(In reply to Brian R. Bondy [:bbondy] from comment #2)
> Yes also in widget/windows/winrt please

it looks clean.

Comment 4

[Matt Brubeck (:mbrubeck)]

Attachment: patch

Comment 5

[Marco Bonardo [:mak]]

Comment on attachment 710329 [details] [diff] [review]
patch

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

Thanks!

::: browser/metro/base/content/bindings/browser.xml
@@ +133,1 @@
>                                                         gFaviconService.FAVICON_LOAD_NON_PRIVATE);

indentation

::: browser/metro/base/content/pages/aboutCertError.xhtml
@@ +24,5 @@
>      <!-- This page currently uses the same favicon as neterror.xhtml.
>           If the location of the favicon is changed for both pages, the
>           FAVICON_ERRORPAGE_URL symbol in toolkit/components/places/src/nsFaviconService.h
>           should be updated. If this page starts using a different favicon
> +         than neterrorm nsFaviconService->SetAndFetchFaviconForPage

while here would be nice to fix "neterrorm" to "neterror," (I actually think this typo is mine, I didn't verify but it really looks my keyboard)

Comment 6

[Matt Brubeck (:mbrubeck)]

http://hg.mozilla.org/projects/elm/rev/633ebf927651

Comment 7

[Matt Brubeck (:mbrubeck)]

Pushed a one-line follow-up to silence errors when the returned URI is null:
https://hg.mozilla.org/projects/elm/rev/5ecb0efc1693