Closed Bug 589324 Opened 9 years ago Closed 9 years ago
Switch-to-Tab after Session Restore does not respect Tab Candy grouping
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:2.0b4) Gecko/20100818 Firefox/4.0b4 After restoring a session with tabs in multiple groupings, Switch-to-tab opens tabs from other groupings in the current displayed grouping. It should be switching the window to display the respective grouping. Steps to Reproduce: 1. Go to www.google.com 2. Open Images | Videos | Maps | News | Books in new tabs with CTRL+Click 3. Open the Tab Candy View and drag News to its own group 4. Click the main group (News should be gone) 5. Type "News" in the location bar and select Switch to Tab (Window should switch to the News grouping) 6. Switch back to the main grouping 7. Quit -> Save & Quit then start Firefox again (Session should restore to the main group) 8. Type "News" in the location bar and select Switch to Tab 9. Open the Tab Candy View and select the main grouping Results: Step 8 > News pops into the current window Step 9 > News pops back into its own grouping Expected: Step 8 > Same behaviour as Step 5 (Window switches to display just News in its own grouping)
This seems like an issue with loading the Panorama sessionstore/group data fast enough.
Assignee: nobody → raymond
Priority: -- → P2
It's because Tab View hasn't loaded yet. Need to do the same thing we do for the "switch group" key combo or the "move tab to group" menu. Note that this can be done before bug 599852, but that that bug will make it faster.
Comment on attachment 484795 [details] [diff] [review] WIP It breaks the browser_visibleTabs.js test. Working on fix the test.
Just updated the browser_visibleTabs.js so the test passes. Sent it to try: 11a8ad41e85b
Comment on attachment 486023 [details] [diff] [review] v1.1 Looks good to me.
Comment on attachment 486023 [details] [diff] [review] v1.1 The tests didn't pass but I couldn't figure out why it causes problem in the tests for bug 580412. @mitcho: could you give me some help please since you wrote the tests? http://tinderbox.mozilla.org/showlog.cgi?log=MozillaTry/1288113133.1288114014.12933.gz s: talos-r3-fed-017 TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/base/content/test/tabview/browser_tabview_bug580412.js | Move away from the edge TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/base/content/test/tabview/browser_tabview_bug580412.js | Moving five pixels: shouldn't snap
Raymond, try adding this to your patch and pushing to try. This fixed this issue for me locally.
Comment on attachment 486023 [details] [diff] [review] v1.1 >+ // make sure we don't trigger the 'first run' behavior >+ prefService.setBoolPref("experienced_first_run", true); >+ // clean up and finish >+ prefService.setBoolPref("experienced_first_run", false); Raymond, also, note that the experienced_first_run should not be overridden in an individual test, as I mentioned on IRC before. :)
Thanks mitcho, your patch works! Passed try!
Comment on attachment 487299 [details] [diff] [review] v1.2 Looks good.
Blocking request was on this bug's dupe in bug 609126. Blocking+ for interaction unexpectedness between two new features that a lot of eyes will be on.
blocking2.0: --- → final+
Comment on attachment 487299 [details] [diff] [review] v1.2 Load balancing review to dietrich now that this is blocking.
Attachment #487299 - Flags: review?(dolske) → review?(dietrich)
Attachment #487299 - Flags: review?(dietrich) → review+
Doesn't need approval as it's blocking. Raymond, please package for check-in.
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.