Closed
Bug 588712
Opened 14 years ago
Closed 14 years ago
After restarting, showing tab groups results in cornered new tabs
Categories
(Firefox Graveyard :: Panorama, defect, P1)
Firefox Graveyard
Panorama
Tracking
(blocking2.0 final+)
VERIFIED
FIXED
Firefox 4.0b7
Tracking | Status | |
---|---|---|
blocking2.0 | --- | final+ |
People
(Reporter: Mardak, Assigned: raymondlee)
References
Details
(Whiteboard: fixed by bug 587990)
Attachments
(1 file, 4 obsolete files)
5.79 KB,
patch
|
Details | Diff | Splinter Review |
If you open new tabs after restarting Firefox and have a tab that hasn't been grouped selected when showing groups, all the new tabs will end up in the corner as ungrouped tabs.
Comment 1•14 years ago
|
||
Nominating as blocking - it's very confusing when this happens, and the ways to get out of it (e.g. draw a group around all the ungrouped tabs piled up in the corner) are not obvious.
blocking2.0: --- → ?
Comment 2•14 years ago
|
||
Agreed; zpao, will this be fixed by any pending work on your end, or does this have nothing to do with how sessionStore is storing TabCandy info? (not an accusation, I'm just forgetting where those bugs are!)
blocking2.0: ? → final+
Comment 4•14 years ago
|
||
This could be fixed with Dietrich's work in bug 588217, so bouncing to him.
Comment 5•14 years ago
|
||
Ed, can you clarify the steps to reproduce? Also, this doesn't sound related to the session restore changes.
Reporter | ||
Comment 6•14 years ago
|
||
1) Group some tabs 2) Restart firefox 3) Open and select a new tab 4) Open Panorama expected: new tab is in a group actual: new tab is in the corner
Comment 7•14 years ago
|
||
Please clarify: are tabs being placed in the bottom right but then slowly moving to their appropriate respective groups (bug 591709, now marked as a duplicate of bug 590268 as that should solve it)? Or are they being placed in the bottom right and then not moving anywhere, in which case their group assignments were actually lost?
Comment 9•14 years ago
|
||
(In reply to comment #7) > Please clarify: are tabs being placed in the bottom right but then slowly > moving to their appropriate respective groups (bug 591709, now marked as a > duplicate of bug 590268 as that should solve it)? Or are they being placed in > the bottom right and then not moving anywhere, in which case their group > assignments were actually lost? I am seeing tabs placed in the bottom right and not moving anywhere - their group assignments were lost. If I open one of the "cornered" tabs, then it appears alone in the tab bar (even if it was together with other tabs were the tab bar before activating Panorama).
Comment 10•14 years ago
|
||
(In reply to comment #9) > I am seeing tabs placed in the bottom right and not moving anywhere - their > group assignments were lost. If I open one of the "cornered" tabs, then it > appears alone in the tab bar (even if it was together with other tabs were the > tab bar before activating Panorama). Thanks for clarifying.
Updated•14 years ago
|
Keywords: helpwanted
Reporter | ||
Comment 11•14 years ago
|
||
You can also trigger this by restarting Firefox, opening a tab, selecting it, pressing ctrl-` (to switch groups).
Reporter | ||
Comment 12•14 years ago
|
||
If you close one of these cornered tabs, you get: tabview assert: must already be linked([object XULElement])@chrome://browser/content/tabview.js:4557 ([object XULElement])@chrome://browser/content/tabview.js:4538 ([object XULElement],[object Event])@chrome://browser/content/tabview.js:4474 ((function (tab) {if (tab.ownerDocument.defaultView != gWindow) {return;}self.update(tab);}),0,[object Array])@resource:///modules/tabview/AllTabs.jsm:127 ([object Event])@resource:///modules/tabview/AllTabs.jsm:125 _tabAttrModified([object XULElement])@chrome://browser/content/tabbrowser.xml:880 updateCurrentBrowser()@chrome://browser/content/tabbrowser.xml:847 onselect([object Event])@chrome://browser/content/browser.xul:1
Reporter | ||
Comment 13•14 years ago
|
||
The stack of how these items end up calling positionNewTabAtBottom(tabItem): GroupItems_newTab([object Object])@chrome://browser/content/tabview.js:3548 TabItem([object XULElement])@chrome://browser/content/tabview.js:4003 TabItems_link([object XULElement])@chrome://browser/content/tabview.js:4619 ([object XULElement],2,[object Array])@chrome://browser/content/tabview.js:4494 TabItems_init()@chrome://browser/content/tabview.js:4490 UI_init()@chrome://browser/content/tabview.js:6267 @chrome://browser/content/tabview.js:7147
Reporter | ||
Comment 14•14 years ago
|
||
Attachment #472799 -
Flags: feedback?(ian)
Comment 15•14 years ago
|
||
Comment on attachment 472799 [details] [diff] [review] wip v1 Does this change actually fix it? It seems a harmless enough change (except for causing redundant work), but doesn't really seem like it should fix the problem. If it does, perhaps it's a symptom of a deeper cause?
Reporter | ||
Comment 16•14 years ago
|
||
The reason why it breaks now is because the newly opened tab is not part of any group, so when activating panorama, there's no active group set (nor active orphan tab) when linking the new tab. The patch relaxes the requirement of the active group being the selected tab to be that of visible tabs.
Updated•14 years ago
|
Priority: -- → P1
Reporter | ||
Comment 17•14 years ago
|
||
I suppose the real fix is to not create these orphan tabs and always put them in a group. And if we don't think we have have an active group, we'll need to somehow pick one.
Comment 18•14 years ago
|
||
(In reply to comment #17) > I suppose the real fix is to not create these orphan tabs and always put them > in a group. And if we don't think we have have an active group, we'll need to > somehow pick one. Agreed. bug 595893
Comment 19•14 years ago
|
||
Comment on attachment 472799 [details] [diff] [review] wip v1 Ok, gotcha; nice catch.
Attachment #472799 -
Flags: review?(dietrich)
Attachment #472799 -
Flags: feedback?(ian)
Attachment #472799 -
Flags: feedback+
Reporter | ||
Comment 20•14 years ago
|
||
Adding test for r?
Assignee: nobody → edilee
Attachment #472799 -
Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #474925 -
Flags: review?(dietrich)
Attachment #472799 -
Flags: review?(dietrich)
Reporter | ||
Comment 21•14 years ago
|
||
Use about: instead of about:config (switched to :config for debugging earlier)
Attachment #474925 -
Attachment is obsolete: true
Attachment #474926 -
Flags: review?(dietrich)
Attachment #474925 -
Flags: review?(dietrich)
Updated•14 years ago
|
Attachment #474926 -
Flags: review?(dietrich)
Attachment #474926 -
Flags: review+
Attachment #474926 -
Flags: approval2.0+
Reporter | ||
Comment 22•14 years ago
|
||
Attachment #474926 -
Attachment is obsolete: true
Reporter | ||
Comment 23•14 years ago
|
||
http://hg.mozilla.org/mozilla-central/rev/a6b06ebcbae2 Instead of only setting the active group on reconnect of the selected tab, do so for any visible tabs.
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Flags: in-testsuite+
Keywords: helpwanted
Resolution: --- → FIXED
Target Milestone: --- → Firefox 4.0b6
Comment 24•14 years ago
|
||
Backed out: http://hg.mozilla.org/mozilla-central/rev/e9fbcd30350b This showed up on Linux debug and Linux 64 debug. The test failure in browser_privatebrowsing_protocolhandler.js showed up on Linux 64 opt. s: talos-r3-fed-007 TEST-UNEXPECTED-FAIL | automationutils.processLeakLog() | leaked 373235 bytes during test execution TEST-UNEXPECTED-FAIL | automationutils.processLeakLog() | leaked 140 instances of AtomImpl with size 20 bytes each (2800 bytes total) TEST-UNEXPECTED-FAIL | automationutils.processLeakLog() | leaked 1 instance of BackstagePass with size 24 bytes TEST-UNEXPECTED-FAIL | automationutils.processLeakLog() | leaked 2 instances of CSSImportRuleImpl with size 48 bytes each (96 bytes total) TEST-UNEXPECTED-FAIL | automationutils.processLeakLog() | leaked 46 instances of CSSImportantRule with size 16 bytes each (736 bytes total) TEST-UNEXPECTED-FAIL | automationutils.processLeakLog() | leaked 11 instances of CSSNameSpaceRuleImpl with size 44 bytes each (484 bytes total) TEST-UNEXPECTED-FAIL | chrome://mochikit/content/browser/browser/components/privatebrowsing/test/browser/browser_privatebrowsing_protocolhandler.js | application timed out after 330 seconds with no output PROCESS-CRASH | chrome://mochikit/content/browser/browser/components/privatebrowsing/test/browser/browser_privatebrowsing_protocolhandler.js | application crashed (minidump found) Thread 0 (crashed) TEST-UNEXPECTED-FAIL | automationutils.processLeakLog() | missing output line for total leaks!
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Comment 26•14 years ago
|
||
This test failure appears to be the same as the one for bug 587990; should probably have the same fix. See Ehsan's comment: https://bugzilla.mozilla.org/show_bug.cgi?id=587990#c45
Comment 27•14 years ago
|
||
Perhaps use the same sort of window close observation Sean just now used in the test for bug 595236?
Updated•14 years ago
|
Assignee: edilee → raymond
Reporter | ||
Comment 28•14 years ago
|
||
Raymond probably already knows, but bug 587990 implements what I have in this patch and more. All that this patch really adds then is a test, but it seems to fail with the test timeout.
Depends on: 587990
Assignee | ||
Comment 29•14 years ago
|
||
Attachment #475104 -
Attachment is obsolete: true
Attachment #475740 -
Flags: feedback?(edilee)
Reporter | ||
Comment 30•14 years ago
|
||
Still not sure why the various attempts to get the test passing aren't working. The main code fix came with the dependent bug so all that's left here is landing the test if it starts working.
Status: REOPENED → RESOLVED
Closed: 14 years ago → 14 years ago
Resolution: --- → FIXED
Whiteboard: fixed by bug 587990
Comment 32•14 years ago
|
||
Let's keep an eye out for this one, because I got the pesky mini-tabs cornered on the top left of my Panorama view after I restarted after applying today's nightly on Mac. It didn't happen on my Windows 7 machine.
Comment 33•14 years ago
|
||
"I got the pesky mini-tabs cornered on the top left of my Panorama view after I restarted after applying today's nightly" -> happened to me yesterday and today on my Win7 machine...
Reporter | ||
Updated•12 years ago
|
Attachment #475740 -
Flags: feedback?(edilee)
Updated•8 years ago
|
Product: Firefox → Firefox Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•