894391 - Remove deprecated PageThumbs.getFileForURL from PageThumbs.jsm

Status: RESOLVED FIXED

(Firefox :: New Tab Page, defect, P3)

Description

[Mike Kaply [:mkaply]]

The PageThumbs deprecation warning says:

PageThumbs.getFileForURL is deprecated. Please use PageThumbs.getFilePathForURL and OS.File

This is incorrect.

It should be:

PageThumbsStorage.getFileForURL is deprecated. Please use PageThumbsStorage.getFilePathForURL and OS.File

I realize this is only for one release, but it is quite confusing.

Comment 1

[Mike Kaply [:mkaply]]

This has been deprecated (and documented wrong) for 23 releases now.

At what point do we remove deprecated things?

Comment 2

[BMO Automation]

Per policy at https://wiki.mozilla.org/Bug_Triage/Projects/Bug_Handling/Bug_Husbandry#Inactive_Bugs. If this bug is not an enhancement request or a bug not present in a supported release of Firefox, then it may be reopened.

Comment 3

[Mike Kaply [:mkaply]]

We should just remove this.

And the tests that reference it.

Comment 4

[Sharon Ho]

Hi, I'm a first time contributor and I would like to help with this. Can I work on it?

Comment 5

[Mike Kaply [:mkaply]]

Yes, you can jsut contrib(In reply to Sharon Ho from comment #4)

Hi, I'm a first time contributor and I would like to help with this. Can I work on it?

Yes, that would be great.

Comment 6

[Sharon Ho]

Does the sanity check that ensures |getFileForURL| points to the same path as |getFilePathForURL| in browser_thumbnails_storage_migrate3.js (lines 77-87) need to be removed as well?

Comment 7

[Mike Kaply [:mkaply]]

Yes, that should be able to be removed as well.

Comment 8

[Sharon Ho]

I've removed the related code and built it to verify there are no errors. Are there any tests I have to run to make sure it's okay?

Comment 9

[Mike Kaply [:mkaply]]

Yes, you would run:

./mach test toolkit/components/thumbnails/test

Comment 10

[Sharon Ho]

The code passed the above tests. Is there anything else to do before I submit a patch? Who do I add as reviewer for the patch?

Comment 11

[Mike Kaply [:mkaply]]

Go ahead and submit the patch. We'll find a reviewer.

Comment 12

[Martin McNickle]

Attachment: Bug 894391 - Remove deprecated PageThumbs.getFileForURL from PageThumbs.jsm r=mkaply

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/fe29ec1ab8c3
Remove deprecated PageThumbs.getFileForURL from PageThumbs.jsm r=mkaply

Comment 14

[Noemi Erli[:noemi_erli]]

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