Closed
Bug 568971
Opened 15 years ago
Closed 13 years ago
Nuke nsIGlobalHistory3
Categories
(Toolkit :: Places, defect)
Toolkit
Places
Tracking
()
RESOLVED
FIXED
mozilla9
People
(Reporter: stechz, Assigned: matjk7)
References
Details
(Keywords: addon-compat, dev-doc-complete, Whiteboard: [inbound])
Attachments
(1 file, 3 obsolete files)
28.73 KB,
patch
|
Details | Diff | Splinter Review |
Since docshell will no longer be requiring the use of GH3, and it was included for places use, we can get rid of the interface now.
Reporter | ||
Updated•15 years ago
|
Updated•15 years ago
|
Assignee: nobody → dougt
Status: NEW → ASSIGNED
Updated•15 years ago
|
Status: ASSIGNED → NEW
Assignee | ||
Comment 2•14 years ago
|
||
Attachment #541619 -
Flags: review?(sdwilsh)
Assignee | ||
Comment 3•13 years ago
|
||
Updated to tip.
Attachment #541619 -
Attachment is obsolete: true
Attachment #549678 -
Flags: review?(mak77)
Attachment #541619 -
Flags: review?(sdwilsh)
Comment 4•13 years ago
|
||
Will check this soon, as a first feedback I think we should wait to land this after next merge, since it's pretty near.
Assignee: nobody → matjk7
Status: NEW → ASSIGNED
OS: Linux → All
Hardware: x86 → All
Comment 5•13 years ago
|
||
Stupid question, but due to the nature of the patch I have to ask it: do the 2 tests you are modifying still pass correctly? test_history_redirects.js may probably work, I'm surprised of test_bug_411966.html. maybe it's already going through the new code, or was just plain wrong.
Assignee | ||
Comment 6•13 years ago
|
||
(In reply to comment #5)
> Stupid question, but due to the nature of the patch I have to ask it: do the
> 2 tests you are modifying still pass correctly? test_history_redirects.js
> may probably work, I'm surprised of test_bug_411966.html. maybe it's already
> going through the new code, or was just plain wrong.
I don't know if the tests are wrong but they do pass: http://tbpl.mozilla.org/?tree=Try&rev=e6ea48e2cb2b
Comment 7•13 years ago
|
||
Comment on attachment 549678 [details] [diff] [review]
patch
Review of attachment 549678 [details] [diff] [review]:
-----------------------------------------------------------------
r=me with the following comment addressed.
I suppose you also need a superreview since you are removing an interface.
For added safety, please land this after we have merged for Firefox 9.
::: toolkit/components/places/nsNavHistory.cpp
@@ +2736,5 @@
> // Normally docshell sends the link visited observer notification for us (this
> // will tell all the documents to update their visited link coloring).
> + // However, for downloads (since we implement nsIDownloadHistory)
> + // this will not happen and we need to send it ourselves.
> + if (newItem && aTransitionType == TRANSITION_DOWNLOAD) {
changing the comment to remove the reference to nsIGlobalHistory is fine, but you should not remove the aIsRedirect check from here, since addVisit is still an exposed API (even if we should try to deprecate it soonish). Also addURI may call into this.
Attachment #549678 -
Flags: review?(mak77) → review+
Assignee | ||
Comment 8•13 years ago
|
||
Review comment addressed.
Attachment #549678 -
Attachment is obsolete: true
Attachment #551222 -
Flags: superreview?
Attachment #551222 -
Flags: review+
Assignee | ||
Updated•13 years ago
|
Attachment #551222 -
Flags: superreview? → superreview?(bzbarsky)
Comment 9•13 years ago
|
||
Comment on attachment 551222 [details] [diff] [review]
patch
sr=me
Attachment #551222 -
Flags: superreview?(bzbarsky) → superreview+
Assignee | ||
Comment 10•13 years ago
|
||
Updated to tip.
Attachment #551222 -
Attachment is obsolete: true
Assignee | ||
Updated•13 years ago
|
Whiteboard: [land for Firefox 9]
Version: unspecified → Trunk
Assignee | ||
Updated•13 years ago
|
Keywords: checkin-needed
Comment 11•13 years ago
|
||
Keywords: checkin-needed
Whiteboard: [land for Firefox 9] → [inbound]
Comment 12•13 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla9
Assignee | ||
Updated•13 years ago
|
Keywords: dev-doc-needed
Updated•13 years ago
|
Keywords: addon-compat
Comment 13•13 years ago
|
||
This was already updated:
https://developer.mozilla.org/en/NsIGlobalHistory3
Removal mentioned on Firefox 9 for developers.
Keywords: dev-doc-needed → dev-doc-complete
You need to log in
before you can comment on or make changes to this bug.
Description
•