Closed Bug 854925 Opened 10 years ago Closed 9 years ago

Remove SetCharsetForURI and GetCharsetForURI from nsINavHistoryService

Categories

(Toolkit :: Places, defect)

Product:
Toolkit
Component:
Places
Type:
defect
Priority:
Not set
Severity:
normal
Points:
1

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla34
Iteration:
34.1

People

(Reporter: raymondlee, Assigned: reznord, Mentored)

References

Details

(Keywords: addon-compat, Whiteboard: [lang=cpp][good first bug])

Attachments

(1 file, 2 obsolete files)

bug854925.patch
4.70 KB, patch
Details | Diff | Splinter Review
Attached patch Remove CPP code (obsolete) — — Splinter Review 
      No description provided.
Depends on: 846635
Depends on: 846644
Blocks: 880922
No longer blocks: 880922
Depends on: 880922
No longer depends on: 846635, 846644
Depreciate warning is added to both method.  We don't use them anymore in the mozilla-central.  Just several add-ons are still using them
we added deprecation in 25, I think we'll remove the code in 27.
Blocks: fxdesktopbacklog
Whiteboard: p=0
No longer blocks: fxdesktopbacklog
Flags: firefox-backlog+
Whiteboard: p=0 → p=1
Some widely (more than 2k users) used add-ons didn't update their code yet:
https://addons.mozilla.org/en-US/firefox/addon/pentadactyl/
https://addons.mozilla.org/en-US/firefox/addon/new-tab-jumpstart/
https://addons.mozilla.org/en-US/firefox/addon/cybersearch/
https://addons.mozilla.org/en-US/firefox/addon/vimperator/

we probably need help from the AMO team to reach their authors and see if they can move to the new APIs, provided they really need to set(and thus enforce)/get the charset.
The replacements are PlacesUtils.setCharsetForURI and PlacesUtils.getCharsetForURI, both return a Promise (this differs from the old API that was directly returning the value).
Flags: needinfo?(jorge)
This should be easy to detect using the mass validations we do for every release. No need to block on notifying the developers.
Flags: needinfo?(jorge)
Keywords: addon-compat
Whiteboard: p=1 → p=1[lang=cpp][mentor=mak]
Whiteboard: p=1[lang=cpp][mentor=mak] → p=1[lang=cpp][mentor=mak][good next bug]
Attachment #729590 - Attachment is patch: true
it's probably just matter of unbitrotting this patch and check there are no consumers in mxr, so an easy one.
Whiteboard: p=1[lang=cpp][mentor=mak][good next bug] → p=1[lang=cpp][mentor=mak][good first bug]
Mentor: mak77
Whiteboard: p=1[lang=cpp][mentor=mak][good first bug] → p=1[lang=cpp][good first bug]
Hi,

I am interested in working on this bug. Can you please assign this bug to me?

Thanks in advance,

Regards,
Anup
Assignee: nobody → allamsetty.anup
Status: NEW → ASSIGNED
Points: --- → 1
Whiteboard: p=1[lang=cpp][good first bug] → [lang=cpp][good first bug]
Removed SetCharsetForURI and GetCharsetForURI from nsINavHistoryService and updated the UUID in nsINavHistoryService.idl
Attachment #8460264 - Flags: review?(mak77)
Keywords: checkin-needed
this still need review right? at least i see no r+ for attachment 8460264 [details] [diff] [review]
Keywords: checkin-needed
Attachment #729590 - Attachment is obsolete: true
Comment on attachment 8460264 [details] [diff] [review]
Removed SetCharsetForURI and GetCharsetForURI and updated the UUID.

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

It looks good!
you just need to add r=mak to the checkin comment, and it will be ready for check-in. no need for further reviews.
Attachment #8460264 - Flags: review?(mak77) → review+
Keywords: checkin-needed
Attached patch bug854925.patch — — Splinter Review 
added r=mak and added the checkin needed also.
Attachment #8460264 - Attachment is obsolete: true
Attachment #8462797 - Flags: review?(mak77)
Comment on attachment 8462797 [details] [diff] [review]
bug854925.patch

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

no need for a further review
Attachment #8462797 - Flags: review?(mak77)
https://hg.mozilla.org/mozilla-central/rev/b70f46a09115
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla34
Iteration: --- → 34.1
QA Whiteboard: [qa?]
QA Whiteboard: [qa?] → [qa-]
You need to log in before you can comment on or make changes to this bug.