Because there is no good way to see if a page has a thumbnail (see bug 818225), we used the API: PageThumbsStorage.getFileForURL in our extension. As part of the rewrite of storage in bug 753768, this API was removed. This change was unnecessary. getFileForURL could have been left behind and the new getFilePathForURL added. I realize this an "internal" API, but it is a JSM file that any extension could have included. Considering this API has survived a couple rewrites of the page thumbs code, it should have stayed.

As Mike said, this function offered functionality that's necessary for some extensions, and that cannot be found in a better way. Please do make an effort to keep APIs stable. RapidReleases are no excuse for lack of stable APIs. Extensions are one of the main reasons why people like Firefox, and unnecessary breakage like this is the very reason why extension authors abandon their extensions. Esp. in this case, it would have been very easy to avoid this breakage. It's literally 3 lines of JS: getFileForURL : function(pageURL) { return new FileUtils.File(PageThumbsStorage.getFilePathForURL(pageURL)); }

Which extensions did we break exactly?

web.de mailcheck. gmx mailcheck. mail.com mailcheck. Their non AMO distributions have a few million users. But there are also AMO versions. Was AMO search to see if there were other consumers of PageThumbs?

(In reply to Michael Kaply (mkaply) from comment #0) > Because there is no good way to see if a page has a thumbnail (see bug > 818225), we used the API: > > PageThumbsStorage.getFileForURL > > in our extension. Did you consider filing a bug to add a "good way"?

(In reply to :Gavin Sharp (use gavin@gavinsharp.com for email) from comment #4) > (In reply to Michael Kaply (mkaply) from comment #0) > Did you consider filing a bug to add a "good way"? Yes as I said, bug 818225. It was filed Dec 4 last year.

Cool! Let's fix that one instead of this one.

New Tab Tools and UnitedTB seem to depend on PageThumbs. I admit that I never thought about searching AMO as this was never meant to be a public API. I guess I can reimplement this with a deprecation warning.

> Cool! Let's fix that one instead of this one. getFileForURL() is a good API, no? It just needs to be declared public. How else would you do the API?

(FWIW, some callers and usescases need access to the actual image, e.g. to make their own thumbnail arrangements, not just a boolean about its existence.)

Well, it is not a good API as it returns a |nsIFile|. We are attempting to get rid of this component, as: - it is generally slow; - it is designed for main thread blocking I/O, which is one of the most important performance bottlenecks we need to fight. Now, we could reimplement this with a deprecation warning and eventually get rid of that API. But it is not here to stay.

It doesn't need to be nsIFile. What's needed is access to the picture, so that I can either use it in a webpage or process its data. I am not sure what the API du jour is, but the reason for introducing and using nsIFile everywhere instead of filepaths as strings was that filepaths were not unique, esp. on Mac OS with 2 volumes of the same name. If that's not a concern and getFilePathForURL() is a good idea, then I have no problem with that, as long as it's public and stable. The main point of this bug was that certain APIs are necessary for exts (no matter whether they are officially public or not), and they should be kept stable. Maybe you can also take a look at your all your APIs, and see which functions may be useful for exts, and declare those public. As seen here, keeping them stable is often not that hard.

Thanks for the efforts and help, BTW!

Well, the "API du jour" is called OS.File: https://developer.mozilla.org/en-US/docs/JavaScript_OS.File - it provides faster, off main thread I/O for javascript. Sorry for not realizing that some add-ons were using this API. I'll work on bug 818225 while waiting for my compilation to complete :)

Same one, minus typo.

I think I'd rather go with Gavin's suggestion and implement the "good way" in bug 818225. PageThumbsStorage is a private API that was never meant to be for add-ons and we shouldn't encourage add-on authors to continue using it. We should rather file a follow-up to put everything storage related into toolkit/components/thumbnails/_Storage.jsm or the like so that people rely on our abstraction and not the implementation itself.

I believe that we should restore getFileForURL for the time being and remove it a few weeks after we have fixed bug 818225.

(In reply to David Rajchenbach Teller [:Yoric] from comment #17) > I believe that we should restore getFileForURL for the time being and remove > it a few weeks after we have fixed bug 818225. Let's do that.

Try: https://tbpl.mozilla.org/?tree=Try&rev=730f19370316

BenB, by the way, I don't think your addon is on AMO. You should uploading it. Among other stuff, this will let us find out more easily whether we break it.

It is: https://addons.mozilla.org/en-US/firefox/addon/webde-mailcheck/?src=search https://addons.mozilla.org/en-US/firefox/addon/mailcom-mailcheck/?src=search

My bad.

Comment on attachment 732843 [details] [diff] [review] Restoring getFileForURL, with a deprecation warning, v2 [Approval Request Comment] Bug caused by (feature/regressing bug #): bug 753768 User impact if declined: we removed a deprecated API but decided we would like to give add-on authors more time to fix their add-ons Testing completed (on m-c, etc.): Risk to taking this patch (and alternatives if risky): low risk String or IDL/UUID changes made by this patch: none

Mozilla/5.0 (Windows NT 6.1; WOW64; rv:23.0) Gecko/20100101 Firefox/23.0 Mozilla/5.0 (X11; Linux x86_64; rv:23.0) Gecko/20100101 Firefox/23.0 Mozilla/5.0 (Macintosh; Intel Mac OS X 10.8; rv:23.0) Gecko/20100101 Firefox/23.0 Verified as fixed the mail.com extension (https://addons.mozilla.org/en-US/firefox/addon/mailcom-mailcheck/?src=search) on Firefox 23 RC 1 (Build ID: 20130730113002). Couldn't verify the web.de extension because my ip address is blocked (even after changing proxies) and I couldn't create an e-mail address; Could anyone with an account please verify web.de addon? Please let me know if there are any other addons/scenarios I should cover.

I can verify this is wokring on web.de

Thanks for your help Alexandra and Mike.