Closed
Bug 656923
Opened 13 years ago
Closed 13 years ago
Tab opened on command-line is in the background behind restored tabs
Categories
(Firefox for Android Graveyard :: General, defect)
Firefox for Android Graveyard
General
Tracking
(Not tracked)
VERIFIED
FIXED
Firefox 6
People
(Reporter: mbrubeck, Assigned: mbrubeck)
References
Details
(Keywords: regression, Whiteboard: [fennec-sessionstore])
Attachments
(1 file, 1 obsolete file)
6.01 KB,
patch
|
mfinkle
:
review+
|
Details | Diff | Splinter Review |
Steps to reproduce: 1. Open a web page in Fennec. 2. Kill Fennec. 3. Open Fennec with a URL on the command line (desktop) for from another app (Android). Expected results: The URL from the command line opens in the foreground tab. Actual results: The URL from the command line opens in a background tab, while a restored tab from the previous session is in the foreground. Looks like this is probably a regression from bug 653091.
Assignee | ||
Comment 1•13 years ago
|
||
Commenting out the code from bug 653091 does not fix this bug.
No longer blocks: 653091
Assignee | ||
Comment 2•13 years ago
|
||
This happens because SessionStore.restoreLastSession now uses NetUtil.asyncFetch, which means its tabs get added after the startup tab. Regression from bug 630398.
Blocks: 630398
Comment 3•13 years ago
|
||
We can use "sessionstore-windows-restored" to listen for when the session is finished being restored and update the selected tab. We already do something like this to kill the dummy tab we need to create: http://mxr.mozilla.org/mozilla-central/source/mobile/chrome/content/browser.js#353 Looks like we just need to tweak the code there a little bit.
Assignee | ||
Comment 4•13 years ago
|
||
This patch uses a different approach (I wrote it before reading your comment) - if there is already an active tab, then we don't create the dummy tab at all, and we don't bring other tabs to the front during restore. If we don't want to modify the nsISessionStore interface, then we could move more of this logic into SessionStore.js. (I thought it was better to let Browser make the decisions about focusing tabs, however.)
Attachment #532252 -
Flags: review?(mark.finkle)
Comment 5•13 years ago
|
||
Comment on attachment 532252 [details] [diff] [review] patch >diff --git a/mobile/chrome/content/browser.js b/mobile/chrome/content/browser.js > let ss = Cc["@mozilla.org/browser/sessionstore;1"].getService(Ci.nsISessionStore); > if (ss.shouldRestore()) { >+ // First open any commandline URLs, except the homepage >+ if (commandURL && commandURL != this.getHomePage()) >+ this.addTab(commandURL, true); >+ Can't we move the dummy-tab code into an "else" block here? Also, use a local bool "bringToFront" instead of using "hasTabs" >+ ss.restoreLastSession(!hasTabs); I'm not a huge fan of this. I agree with you about letting browser.js handle the selection. I suppose it works for now. > addTab: function browser_addTab(aURI, aBringFront, aOwner, aParams) { >+ debugger; remove r-, just to see the nits fixed and the minor refactor
Attachment #532252 -
Flags: review?(mark.finkle) → review-
Assignee | ||
Comment 6•13 years ago
|
||
Addressed review comments
Attachment #532252 -
Attachment is obsolete: true
Attachment #532437 -
Flags: review?(mark.finkle)
Updated•13 years ago
|
Attachment #532437 -
Flags: review?(mark.finkle) → review+
Assignee | ||
Comment 7•13 years ago
|
||
http://hg.mozilla.org/mozilla-central/rev/992f91cd375d
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Assignee | ||
Updated•13 years ago
|
Target Milestone: --- → Firefox 6
Comment 8•13 years ago
|
||
Verified fixed on build: Mozilla /5.0 (Android;Linux armv7l;rv:6.0a1) Gecko/20110517 Firefox/6.0a1 Fennec/6.0a1 Device: LG Optimus 2X (Android 2.2)
Status: RESOLVED → VERIFIED
You need to log in
before you can comment on or make changes to this bug.
Description
•