Closed
Bug 854925
Opened 11 years ago
Closed 9 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)
4.70 KB,
patch
|
Details | Diff | Splinter Review |
No description provided.
Reporter | ||
Updated•11 years ago
|
Reporter | ||
Updated•11 years ago
|
Reporter | ||
Comment 1•10 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•10 years ago
|
||
we added deprecation in 25, I think we'll remove the code in 27.
Updated•10 years ago
|
Blocks: fxdesktopbacklog
Whiteboard: p=0
Updated•10 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•9 years ago
|
Mentor: mak77
Whiteboard: p=1[lang=cpp][mentor=mak][good first bug] → p=1[lang=cpp][good first bug]
Assignee | ||
Comment 6•9 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•9 years ago
|
Assignee: nobody → allamsetty.anup
Status: NEW → ASSIGNED
Updated•9 years ago
|
Points: --- → 1
Whiteboard: p=1[lang=cpp][good first bug] → [lang=cpp][good first bug]
Assignee | ||
Comment 7•9 years ago
|
||
Removed SetCharsetForURI and GetCharsetForURI from nsINavHistoryService and updated the UUID in nsINavHistoryService.idl
Attachment #8460264 -
Flags: review?(mak77)
Assignee | ||
Comment 8•9 years ago
|
||
The try request link: https://tbpl.mozilla.org/?tree=Try&rev=362aa575034c
Assignee | ||
Updated•9 years ago
|
Keywords: checkin-needed
Comment 9•9 years ago
|
||
this still need review right? at least i see no r+ for attachment 8460264 [details] [diff] [review]
Keywords: checkin-needed
Updated•9 years ago
|
Attachment #729590 -
Attachment is obsolete: true
Comment 10•9 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•9 years ago
|
Keywords: checkin-needed
Assignee | ||
Comment 11•9 years ago
|
||
added r=mak and added the checkin needed also.
Attachment #8460264 -
Attachment is obsolete: true
Attachment #8462797 -
Flags: review?(mak77)
Comment 12•9 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•9 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/b70f46a09115
Keywords: checkin-needed
Comment 14•9 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/b70f46a09115
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla34
Updated•9 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
•