Closed Bug 839034 Opened 8 years ago Closed 7 years ago
Page As XXX methods to ns INav History Service
these methods are currently spread half in nsIBrowserHistory and half in nsINavHistoryService, we should just move all of them to nsINavHistoryService.
Attachment #711723 - Flags: review?(mak77)
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
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+
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.
Rev the UUIDs as suggested in comment 3
please see also comment 2
(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)
Attachment #711947 - Flags: review?(mak77) → review+
Attachment #711929 - Flags: superreview?(gavin.sharp) → superreview+
the Metro branch has been merged to central some days ago, so both patches can land in inbound
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.
Attachment #718805 - Attachment is obsolete: true
Target Milestone: --- → mozilla22
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
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.
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.