Closed
Bug 587043
Opened 14 years ago
Closed 14 years ago
Closing the tabcandy window closes the browser
Categories
(Firefox Graveyard :: Panorama, defect, P2)
Firefox Graveyard
Panorama
Tracking
(blocking2.0 betaN+)
VERIFIED
FIXED
Firefox 4.0
Tracking | Status | |
---|---|---|
blocking2.0 | --- | betaN+ |
People
(Reporter: martijn.martijn, Assigned: raymondlee)
References
Details
(Keywords: dataloss, Whiteboard: [Aug-13-testday][good first bug][interaction][b8])
Attachments
(1 file, 2 obsolete files)
4.51 KB,
patch
|
Details | Diff | Splinter Review |
(I'm on Windows 7 using the classic theme).
When I click on the close button of the tab candy window, the whole browser closes.
I would expect the tab candy window to disappear only.
Updated•14 years ago
|
OS: Windows 7 → All
Hardware: x86 → All
Version: unspecified → Trunk
Updated•14 years ago
|
Component: Tabbed Browser → TabCandy
QA Contact: tabbed.browser → tabcandy
Comment 1•14 years ago
|
||
I guess it should be expected that closing the window closes the app (tab candy isn't a separate window, it takes over the browser window). But it should be more obvious that tab candy isn't a separate window and how you can get out of it. Based on that, while this isn't exactly a dupe, I think it's kind of the same issue as bug 586990.
agree, this is a big pita
https://bugzilla.mozilla.org/show_bug.cgi?id=588355
:(
This causes dataloss when sessionstore hasn't saved info recently and on the forms where the information isn't saved. I run into this issue constantly. Requesting blocking because I truly believe a lot of users are going to have the same issue. When you go into panorama mode it seems as if this is a new window due to the chrome disappearing and clicking the big red X in the top right corner seems and really should be the only logical way to exit this mode.
blocking2.0: --- → ?
Keywords: dataloss
Comment 4•14 years ago
|
||
I don't think this blocks now that bug 588999 has created a tab-candy exit hatch.
blocking2.0: ? → -
Rerequesting blocking status. Please consider that the big red X catches peoples eyes and Escape isn't that discoverable. Users are going to be clicking this new button that shows up on their toolbars and will look for a way to get out of it and that big red X looks like the best way to do so. Clicking so it going to close the browser and lose anything that hasn't been saved (due to either users changing the time between saves or because they just happened to hit the browser close button a few seconds before the next save). Remember there are also users that have sessionstore disabled.
This wouldn't be an issue if the chrome and content area was replaced and the panorama stuff was just shown in the content area or if it was discoverable that the panorama button at the top right had some sort of label or an icon that actually made sense, like a close button.
blocking2.0: - → ?
Updated•14 years ago
|
Priority: -- → P2
Comment 7•14 years ago
|
||
Given the number of people I've observed going for the close window to get out, as well as the anecdotal evidence closing the window what Tab Candy is open should return you to the previously active tab.
Requesting blocking.
Assignee: aza → ian
Whiteboard: [Aug-13-testday] → [Aug-13-testday][good first bug][interaction][b8]
Target Milestone: --- → Firefox 4.0
Assignee | ||
Updated•14 years ago
|
Assignee: ian → raymond
Assignee | ||
Updated•14 years ago
|
Status: NEW → ASSIGNED
Assignee | ||
Updated•14 years ago
|
Attachment #476713 -
Attachment is patch: true
Attachment #476713 -
Attachment mime type: application/octet-stream → text/plain
Assignee | ||
Comment 9•14 years ago
|
||
A file is missing in the last patch.
Attachment #476713 -
Attachment is obsolete: true
Attachment #476713 -
Flags: feedback?(ian)
Assignee | ||
Comment 10•14 years ago
|
||
(In reply to comment #9)
> Created attachment 476758 [details] [diff] [review]
> v1
>
Passed Try.
Assignee | ||
Updated•14 years ago
|
Attachment #476758 -
Flags: feedback?(ian)
Comment 11•14 years ago
|
||
Comment on attachment 476758 [details] [diff] [review]
v1
Looks good.
Attachment #476758 -
Flags: review?(dietrich)
Attachment #476758 -
Flags: feedback?(ian)
Attachment #476758 -
Flags: feedback+
Updated•14 years ago
|
blocking2.0: ? → betaN+
Comment 12•14 years ago
|
||
Comment on attachment 476758 [details] [diff] [review]
v1
this is an improvement, but i feel like it's not the best solution since it results in things like the window-close keyboard shortcut not actually closing the window the first time. this not only affects people who want to close the window, but also extensions who might depend on closing the window to actually close the window, regardless of what's visible in it.
r=me, but i think it's a band-aid.
Attachment #476758 -
Flags: review?(dietrich) → review+
Assignee | ||
Comment 13•14 years ago
|
||
Dietrich: do you know where I can method which is first being called when the x button is pressed? I know the BrowserTryToCloseWindow() gets called but I couldn't find the code which triggers the action. Is the x button defined in XUL?
Comment 14•14 years ago
|
||
Aza asked me to comment, so here we go.
Raymond: I think the x button is defined in XUL (I didn't use to think that, but with the firefox menu I think it must be.)
http://mxr.mozilla.org/mozilla-central/source/browser/base/content/browser.xul#807
That's just hooked up to cmd_closeWindow which is hooked up to BrowserTryToCloseWindow
Assignee | ||
Comment 15•14 years ago
|
||
(In reply to comment #14)
> Aza asked me to comment, so here we go.
>
> Raymond: I think the x button is defined in XUL (I didn't use to think that,
> but with the firefox menu I think it must be.)
> http://mxr.mozilla.org/mozilla-central/source/browser/base/content/browser.xul#807
>
That line is only included when MENUBAR_CAN_AUTOHIDE is true, therefore, it's the x button when the menubar is hidden on Windows, not the one we see in all platforms.
http://mxr.mozilla.org/mozilla-central/source/browser/base/content/browser.xul#439
> That's just hooked up to cmd_closeWindow which is hooked up to
> BrowserTryToCloseWindow
Comment 16•14 years ago
|
||
(In reply to comment #15)
> That line is only included when MENUBAR_CAN_AUTOHIDE is true, therefore, it's
> the x button when the menubar is hidden on Windows, not the one we see in all
> platforms.
> http://mxr.mozilla.org/mozilla-central/source/browser/base/content/browser.xul#439
Be that as it may, if we're replicating the window close button in once place, it's relatively safe to assume that that goes down a common path. I would start looking there.
Assignee | ||
Comment 17•14 years ago
|
||
Ideally, we would want to go back to the normal browser mode from Panorama UI only when user clicks on the window x button. The patch adds code to BrowserTryToCloseWindow() so even user uses File > Close Window or window-close shortcut as mentioned in #comment 12, it would go to the normal browser mode rather than closing the window. The window x button is not part of the XUL land so if we want to the Panorama UI back to browser mode behaviour, we would need to look into deeper. Aza/Ian: Is the behaviour implemented in the current patch OK?
Comment 18•14 years ago
|
||
Actually, I think we want the same behavior (window exits Panorama UI) whether you click the close button, hit the close key combo, or select close from the menu. The only thing on Dietrich's list I'd be concerned about is add-ons, but only if it's relatively straightforward to make an exception for them.
Comment 19•14 years ago
|
||
(In reply to comment #18)
> Actually, I think we want the same behavior (window exits Panorama UI) whether
> you click the close button, hit the close key combo, or select close from the
> menu. The only thing on Dietrich's list I'd be concerned about is add-ons, but
> only if it's relatively straightforward to make an exception for them.
Raymond, given this, is there more to be done on this patch?
Assignee | ||
Updated•14 years ago
|
Attachment #476758 -
Flags: approval2.0?
Assignee | ||
Updated•14 years ago
|
Attachment #476758 -
Flags: approval2.0?
Assignee | ||
Comment 20•14 years ago
|
||
Attachment #476758 -
Attachment is obsolete: true
Assignee | ||
Comment 21•14 years ago
|
||
(In reply to comment #19)
> (In reply to comment #18)
> > Actually, I think we want the same behavior (window exits Panorama UI) whether
> > you click the close button, hit the close key combo, or select close from the
> > menu. The only thing on Dietrich's list I'd be concerned about is add-ons, but
> > only if it's relatively straightforward to make an exception for them.
>
> Raymond, given this, is there more to be done on this patch?
There is no more to be done on this.
Assignee | ||
Updated•14 years ago
|
Keywords: checkin-needed
Comment 22•14 years ago
|
||
Comment 23•14 years ago
|
||
Verified fixed using Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0b8pre) Gecko/20101013 Firefox/4.0b8pre
Thank you!!
Status: RESOLVED → VERIFIED
Updated•9 years ago
|
Product: Firefox → Firefox Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•