Closed Bug 613530 Opened 11 years ago Closed 10 years ago

Fix a typo in nsSessionStore.js


(Firefox :: Session Restore, defect)

Not set



Firefox 8


(Reporter: tabutils+bugzilla, Assigned: tabutils+bugzilla)


(Whiteboard: [qa-])


(1 file, 1 obsolete file)

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; zh-CN; rv: Gecko/20101026 Firefox/3.6.12 (.NET CLR 3.5.30729)
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; zh-CN; rv: Gecko/20101026 Firefox/3.6.12 (.NET CLR 3.5.30729)

browser.loadURI(tabData.userTypedValue, null, null, true) is a wrong call.

For browser, we have APIs:
function loadURI(aURI, aReferrerURI, aCharset)
function loadURIWithFlags(aURI, aFlags, aReferrerURI, aCharset, aPostData)

For tabbrowser, we have APIs:
function loadURI(uri, referrer, postData, allowThirdPartyFixup)

Reproducible: Always
Version: unspecified → Trunk
True. It's not going to be harmful passing an extra param, but might as well fix it.

Hmm, actually we might want to have this do the same thing as loadURI from browser.js and call loadURIWithFlags directly so that we get the third party fixup flag set.
Version: unspecified → Trunk
Yes, please set the third party fixup flag.
Attached patch patch (obsolete) — Splinter Review
Attachment #536842 - Flags: review?(paul)
Attached patch patch v2Splinter Review
Attachment #536842 - Attachment is obsolete: true
Attachment #536844 - Flags: review?(paul)
Attachment #536842 - Flags: review?(paul)
Comment on attachment 536844 [details] [diff] [review]
patch v2

>diff --git a/browser/components/sessionstore/src/nsSessionStore.js b/browser/components/sessionstore/src/nsSessionStore.js
>--- a/browser/components/sessionstore/src/nsSessionStore.js
>+++ b/browser/components/sessionstore/src/nsSessionStore.js
>@@ -2911,18 +2911,21 @@ SessionStoreService.prototype = {
>     if (tabData.userTypedValue) {
>       browser.userTypedValue = tabData.userTypedValue;
>       if (tabData.userTypedClear) {
>         // Make it so that we'll enter restoreDocument on page load. We will
>         // fire SSTabRestored from there. We don't have any form data to restore
>         // so we can just set the URL to null.
>         browser.__SS_restore_data = { url: null };
>         browser.__SS_restore_tab = aTab;
>+        if (didStartLoad)
>+          browser.stop();

I guess it doesn't hurt, but is there any particular reason you're calling stop here?
Parallel loading might lead to some abnormality? I'm not sure, but I read that we often stop "about:blank" loading.
ithinc - do you have a name you'd rather have in the commit message or is ithinc ok?
Assignee: nobody → ithinc
Ever confirmed: true
Whiteboard: [fixed-in-fx-team]
Closed: 10 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → Firefox 8
Whiteboard: [qa-]
You need to log in before you can comment on or make changes to this bug.