Closed
Bug 839034
Opened 11 years ago
Closed 11 years ago
move markPageAsXXX methods to nsINavHistoryService
Categories
(Toolkit :: Places, defect)
Tracking
()
RESOLVED
FIXED
mozilla22
People
(Reporter: mak, Assigned: raymondlee)
References
Details
(Keywords: addon-compat)
Attachments
(1 file, 4 obsolete files)
12.58 KB,
patch
|
Details | Diff | Splinter Review |
these methods are currently spread half in nsIBrowserHistory and half in nsINavHistoryService, we should just move all of them to nsINavHistoryService.
Assignee | ||
Updated•11 years ago
|
Assignee: nobody → raymond
Assignee | ||
Updated•11 years ago
|
Status: NEW → ASSIGNED
Assignee | ||
Comment 1•11 years ago
|
||
https://tbpl.mozilla.org/?tree=Try&rev=27d729127f07
Attachment #711723 -
Flags: review?(mak77)
Reporter | ||
Comment 2•11 years ago
|
||
we need also an additional patch for Metro elm project branch, that removes nsIBrowserHistory from here: http://mxr.mozilla.org/projects-central/source/elm/browser/metro/base/content/browser-scripts.js#57
Reporter | ||
Comment 3•11 years ago
|
||
Comment on attachment 711723 [details] [diff] [review] v1 Review of attachment 711723 [details] [diff] [review]: ----------------------------------------------------------------- once these comments are fixed we will need a superreview here, gavin may likely do it. Luckily the ClassInfo on history and the PlacesUIUtils helpers help us with compatibility, I'm now going to check add-ons mxr for breaking usage. ::: toolkit/components/places/nsIBrowserHistory.idl @@ -108,5 @@ > - * > - * @param aURI > - * URI of the page to be marked. > - */ > - void markPageAsFollowedLink(in nsIURI aURI); you must rev the UUID here ::: toolkit/components/places/nsINavHistoryService.idl @@ +1280,5 @@ > + * > + * @param aURI > + * URI of the page to be marked. > + */ > + void markPageAsFollowedLink(in nsIURI aURI); as well as here
Attachment #711723 -
Flags: review?(mak77) → review+
Reporter | ||
Comment 4•11 years ago
|
||
As I suspected, the classInfo and PlacesUtils here makes so that most of the add-ons will keep working without changes, I found a couple that are getting a handle through globalhistory2 and while this bug won't break them, bug 838874 will regardless.
Reporter | ||
Updated•11 years ago
|
Keywords: addon-compat
Assignee | ||
Comment 5•11 years ago
|
||
Rev the UUIDs as suggested in comment 3
Attachment #711723 -
Attachment is obsolete: true
Attachment #711929 -
Flags: superreview?(gavin.sharp)
Assignee | ||
Comment 7•11 years ago
|
||
(In reply to Marco Bonardo [:mak] from comment #2) > we need also an additional patch for Metro elm project branch, that removes > nsIBrowserHistory from here: > http://mxr.mozilla.org/projects-central/source/elm/browser/metro/base/ > content/browser-scripts.js#57 Address this.
Attachment #711947 -
Flags: review?(mak77)
Reporter | ||
Updated•11 years ago
|
Attachment #711947 -
Flags: review?(mak77) → review+
Updated•11 years ago
|
Attachment #711929 -
Flags: superreview?(gavin.sharp) → superreview+
Assignee | ||
Comment 8•11 years ago
|
||
Attachment #711929 -
Attachment is obsolete: true
Assignee | ||
Comment 9•11 years ago
|
||
Attachment #711947 -
Attachment is obsolete: true
Reporter | ||
Comment 10•11 years ago
|
||
the Metro branch has been merged to central some days ago, so both patches can land in inbound
Reporter | ||
Comment 11•11 years ago
|
||
Ah I see you already fixed the usage in central, then probably we don't need the metro patch, from what I see they just merge the branch with central periodically.
Assignee | ||
Comment 12•11 years ago
|
||
Passed try https://tbpl.mozilla.org/?tree=Try&rev=db8b2eb00be8
Keywords: checkin-needed
Whiteboard: c
Reporter | ||
Updated•11 years ago
|
Attachment #718805 -
Attachment is obsolete: true
Reporter | ||
Comment 13•11 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/e73692553d1f
Comment 14•11 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/e73692553d1f
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Comment 15•11 years ago
|
||
Just FYI: I updated https://developer.mozilla.org/en-US/docs/XPCOM_Interface_Reference/nsINavHistoryService and https://developer.mozilla.org/en-US/docs/XPCOM_Interface_Reference/nsIBrowserHistory to reflect the interface changes.
Reporter | ||
Comment 16•11 years ago
|
||
thanks, I indeed forgot to set dev-doc-needed :(
You need to log in
before you can comment on or make changes to this bug.
Description
•