Closed
Bug 854925
Opened 12 years ago
Closed 10 years ago
Remove SetCharsetForURI and GetCharsetForURI from nsINavHistoryService
Categories
(Toolkit :: Places, defect)
Toolkit
Places
Tracking
()
People
(Reporter: raymondlee, Assigned: reznord, Mentored)
References
Details
(Keywords: addon-compat, Whiteboard: [lang=cpp][good first bug])
Attachments
(1 file, 2 obsolete files)
(deleted),
patch
|
Details | Diff | Splinter Review |
No description provided.
Reporter | ||
Updated•11 years ago
|
Reporter | ||
Updated•11 years ago
|
Reporter | ||
Comment 1•11 years ago
|
||
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
Comment 2•11 years ago
|
||
we added deprecation in 25, I think we'll remove the code in 27.
Updated•11 years ago
|
Blocks: fxdesktopbacklog
Whiteboard: p=0
Updated•11 years ago
|
Comment 3•10 years ago
|
||
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)
Comment 4•10 years ago
|
||
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
Updated•10 years ago
|
Whiteboard: p=1 → p=1[lang=cpp][mentor=mak]
Updated•10 years ago
|
Whiteboard: p=1[lang=cpp][mentor=mak] → p=1[lang=cpp][mentor=mak][good next bug]
Updated•10 years ago
|
Attachment #729590 -
Attachment is patch: true
Comment 5•10 years ago
|
||
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]
Updated•10 years ago
|
Mentor: mak77
Whiteboard: p=1[lang=cpp][mentor=mak][good first bug] → p=1[lang=cpp][good first bug]
Assignee | ||
Comment 6•10 years ago
|
||
Hi,
I am interested in working on this bug. Can you please assign this bug to me?
Thanks in advance,
Regards,
Anup
Updated•10 years ago
|
Assignee: nobody → allamsetty.anup
Status: NEW → ASSIGNED
Updated•10 years ago
|
Points: --- → 1
Whiteboard: p=1[lang=cpp][good first bug] → [lang=cpp][good first bug]
Assignee | ||
Comment 7•10 years ago
|
||
Removed SetCharsetForURI and GetCharsetForURI from nsINavHistoryService and updated the UUID in nsINavHistoryService.idl
Attachment #8460264 -
Flags: review?(mak77)
Assignee | ||
Comment 8•10 years ago
|
||
The try request link:
https://tbpl.mozilla.org/?tree=Try&rev=362aa575034c
Assignee | ||
Updated•10 years ago
|
Keywords: checkin-needed
Comment 9•10 years ago
|
||
this still need review right? at least i see no r+ for attachment 8460264 [details] [diff] [review]
Keywords: checkin-needed
Updated•10 years ago
|
Attachment #729590 -
Attachment is obsolete: true
Comment 10•10 years ago
|
||
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+
Assignee | ||
Updated•10 years ago
|
Keywords: checkin-needed
Assignee | ||
Comment 11•10 years ago
|
||
added r=mak and added the checkin needed also.
Attachment #8460264 -
Attachment is obsolete: true
Attachment #8462797 -
Flags: review?(mak77)
Comment 12•10 years ago
|
||
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)
Comment 13•10 years ago
|
||
Keywords: checkin-needed
Comment 14•10 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla34
Updated•10 years ago
|
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.
Description
•