Last Comment Bug 613530 - Fix a typo in nsSessionStore.js
: Fix a typo in nsSessionStore.js
Product: Firefox
Classification: Client Software
Component: Session Restore (show other bugs)
: Trunk
: All All
: -- normal (vote)
: Firefox 8
Assigned To: ithinc
: Mike de Boer [:mikedeboer]
Depends on:
  Show dependency treegraph
Reported: 2010-11-19 10:17 PST by ithinc
Modified: 2011-09-01 15:58 PDT (History)
5 users (show)
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

patch (1.25 KB, patch)
2011-06-02 02:32 PDT, ithinc
no flags Details | Diff | Splinter Review
patch v2 (1.33 KB, patch)
2011-06-02 02:43 PDT, ithinc
paul: review+
Details | Diff | Splinter Review

Description ithinc 2010-11-19 10:17:27 PST
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
Comment 2 Paul O'Shannessy [:zpao] (not reading much bugmail, email directly) 2010-11-29 11:24:36 PST
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.
Comment 3 ithinc 2011-04-30 16:21:53 PDT
Yes, please set the third party fixup flag.
Comment 4 ithinc 2011-06-02 02:32:47 PDT
Created attachment 536842 [details] [diff] [review]
Comment 5 ithinc 2011-06-02 02:43:18 PDT
Created attachment 536844 [details] [diff] [review]
patch v2
Comment 6 Paul O'Shannessy [:zpao] (not reading much bugmail, email directly) 2011-06-21 12:48:33 PDT
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?
Comment 7 ithinc 2011-06-21 21:38:01 PDT
Parallel loading might lead to some abnormality? I'm not sure, but I read that we often stop "about:blank" loading.
Comment 8 Paul O'Shannessy [:zpao] (not reading much bugmail, email directly) 2011-07-06 10:23:14 PDT
ithinc - do you have a name you'd rather have in the commit message or is ithinc ok?
Comment 9 Paul O'Shannessy [:zpao] (not reading much bugmail, email directly) 2011-07-11 15:40:05 PDT
Comment 10 Tim Taubert [:ttaubert] 2011-07-13 15:29:40 PDT

Note You need to log in before you can comment on or make changes to this bug.