Closed
Bug 642355
Opened 14 years ago
Closed 14 years ago
"You're about to close 2 tabs..." dialog when closing popup window; triggered by Print Preview
Categories
(Firefox :: Tabbed Browser, defect)
Firefox
Tabbed Browser
Tracking
()
VERIFIED
FIXED
Firefox 5
People
(Reporter: MatsPalmgren_bugz, Assigned: dao)
References
()
Details
(Keywords: regression, reproducible)
Attachments
(1 file, 1 obsolete file)
(deleted),
patch
|
Gavin
:
review+
|
Details | Diff | Splinter Review |
STEPS TO REPRODUCE
1. load http://www.mapquest.com/
2. enter in the "Search For" text field: tour eiffel, paris
3. click the "Get Map" button
4. click the Print button (above the "Search For")
5. in the new window: Firefox->Print->Print Preview
6. in Print Preview toolbar, click the Close button
Note that the URL bar that was present before Print Preview is now gone
7. click the X button in the upper right corner to close the popup window
=> "You're about to close 2 tabs..." dialog appears
EXPECTED RESULTS
URL bar should be restored when exiting Print Preview.
No warning dialog about 2 tabs.
PLATFORMS AND BUILDS TESTED
Bug occurs in Firefox 4 RC1 on Windows 7.
Bug does not occur in Firefox nightly build 2011-03-09 on Linux.
Comment 1•14 years ago
|
||
The issue here is the menubar. If it is shown you will not see this problem. If you disable the menubar on Linux we don't even show the firefox button for popups so I can't find a way to check this regression on Linux.
The steps pass with Firefox 3.6 and the menu bar hidden.
Keywords: regression,
regressionwindow-wanted
Summary: "You're about to close 2 tabs..." dialog when closing popup window; triggered by Print Preview → "You're about to close 2 tabs..." dialog when closing popup window; triggered by Print Preview and menubar hidden
Comment 2•14 years ago
|
||
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0b13pre) Gecko/20110318 Firefox/4.0b13pre ID:20110318071849
Error in error console are as follows:
*After Step 5(Enter Print Preview):
Error: newBrowser is undefined
Source file: chrome://browser/content/tabbrowser.xml
Line: 867
Error: content is null
Source file: chrome://browser/content/browser.js
Line: 5556
*After Step6(Exit Print Preview):
Error: window.content is null
Source file: chrome://global/content/printUtils.js
Line: 265
*After Step 7 and Chose Close Tabs(Close the popup window):
Error: aWindow.content is null
Source file: resource://gre/components/nsSessionStore.js
Line: 845
Comment 3•14 years ago
|
||
Regression window:
Works:
http://hg.mozilla.org/mozilla-central/rev/528a9b4f475d
Mozilla/5.0 (Windows; U; Windows NT 6.1; en-US; rv:1.9.3a4pre) Gecko/20100317
Minefield/3.7a4pre ID:20100317002903
Fails:
http://hg.mozilla.org/mozilla-central/rev/11798edae90d
Mozilla/5.0 (Windows; U; Windows NT 6.1; en-US; rv:1.9.3a4pre) Gecko/20100317
Minefield/3.7a4pre ID:20100317003949
Pushlog:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=528a9b4f475d&tochange=11798edae90d
Candidate Bug:
Bug 347930 - Tab strip should be a toolbar instead
Blocks: 347930
Keywords: regressionwindow-wanted
Assignee | ||
Comment 4•14 years ago
|
||
This bug is caused by browser.css hiding the tabs toolbar:
http://hg.mozilla.org/mozilla-central/annotate/8618a149ea2e/browser/base/content/browser.css#l319
This patch prevents this and lets updateVisiblity hide the toolbar in popups regardless of browser.tabs.autoHide -- this was already supposed to work this way, but apparently window.toolbar.visible ended up on the wrong line, where it makes no sense.
Assignee | ||
Comment 5•14 years ago
|
||
Also, this happens regardless of whether the menu bar is hidden.
Summary: "You're about to close 2 tabs..." dialog when closing popup window; triggered by Print Preview and menubar hidden → "You're about to close 2 tabs..." dialog when closing popup window; triggered by Print Preview
Assignee | ||
Comment 6•14 years ago
|
||
And on Linux, presumably OS X too.
OS: Windows 7 → All
Hardware: x86 → All
Assignee | ||
Comment 7•14 years ago
|
||
Last but not least, this prevents nsSessionStore.js from handling the closed popup; it sticks around in the session and reappears when restoring that session.
Severity: minor → normal
Comment 8•14 years ago
|
||
(In reply to comment #4)
> This bug is caused by browser.css hiding the tabs toolbar:
> http://hg.mozilla.org/mozilla-central/annotate/8618a149ea2e/browser/base/content/browser.css#l319
How is print preview involved? Why does it cause exceptions like those in comment 2, or the alert about 2 tabs from comment 0?
Assignee | ||
Comment 9•14 years ago
|
||
(In reply to comment #8)
> (In reply to comment #4)
> > This bug is caused by browser.css hiding the tabs toolbar:
> > http://hg.mozilla.org/mozilla-central/annotate/8618a149ea2e/browser/base/content/browser.css#l319
>
> How is print preview involved? Why does it cause exceptions like those in
> comment 2, or the alert about 2 tabs from comment 0?
Print preview adds a tab, but that tab doesn't get an xbl binding applied, so things like setting tab.selected fail.
Comment 10•14 years ago
|
||
(In reply to comment #4)
> This bug is caused by browser.css hiding the tabs toolbar:
Oh - "... thus preventing XBL binding attachment and breaking pretty much all uses of tabContainer" was the crucial part I missed.
> lets updateVisiblity hide the toolbar in popups
> regardless of browser.tabs.autoHide
How is this related?
Comment 11•14 years ago
|
||
Comment on attachment 521640 [details] [diff] [review]
patch
>diff --git a/browser/base/content/browser.xul b/browser/base/content/browser.xul
> <toolbar id="TabsToolbar"
>+ class="toolbar-primary chromeclass-toolbar"
Won't chromeclass-toolbar result in window[chromehidden~="location"][chromehidden~="toolbar"] windows being similarly busted (per http://hg.mozilla.org/mozilla-central/annotate/8618a149ea2e/toolkit/content/xul.css#l57)?
Assignee | ||
Comment 12•14 years ago
|
||
(In reply to comment #10)
> > lets updateVisiblity hide the toolbar in popups
> > regardless of browser.tabs.autoHide
>
> How is this related?
Without it, the toolbar wouldn't hide in popups.
(In reply to comment #11)
> > <toolbar id="TabsToolbar"
> >+ class="toolbar-primary chromeclass-toolbar"
>
> Won't chromeclass-toolbar result in
> window[chromehidden~="location"][chromehidden~="toolbar"] windows being
> similarly busted (per
> http://hg.mozilla.org/mozilla-central/annotate/8618a149ea2e/toolkit/content/xul.css#l57)?
Apparently (except that [chromehidden~="location"] won't match unless a user toggled dom.disable_window_open_feature.location)
Assignee | ||
Comment 13•14 years ago
|
||
removed chromeclass-toolbar
Attachment #521640 -
Attachment is obsolete: true
Attachment #521640 -
Flags: review?(gavin.sharp)
Attachment #522951 -
Flags: review?(gavin.sharp)
Comment 14•14 years ago
|
||
Comment on attachment 522951 [details] [diff] [review]
patch v2
Is toolkit/themes/pinstripe/global/toolbar.css specifying a min-height of 24px for .toolbar-primary going to cause problems? I guess our browser.css gives it a height of 26px, so it should be OK (though maybe we should get rid of that toolkit rule for other reasons).
Attachment #522951 -
Flags: review?(gavin.sharp) → review+
Assignee | ||
Comment 15•14 years ago
|
||
(In reply to comment #14)
> I guess our browser.css gives it a height of 26px, so it should be OK
Yes
> (though maybe we should get rid of that toolkit rule for other reasons).
Yeah, I don't think it belongs there.
Assignee | ||
Comment 16•14 years ago
|
||
Whiteboard: fixed-in-cedar
Comment 17•14 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Whiteboard: fixed-in-cedar
Target Milestone: --- → Firefox4.2
Comment 18•14 years ago
|
||
Verified fixed on Mozilla/5.0 (Windows NT 6.1; rv:2.2a1pre) Gecko/20110404 Firefox/4.2a1pre
Status: RESOLVED → VERIFIED
Assignee | ||
Updated•14 years ago
|
Target Milestone: Firefox5 → Firefox 5
Comment 19•13 years ago
|
||
I still have this on Firefox 6 beta.
Closing a popup gives the message mentionned, neither ok nor cancel button do anything
You need to log in
before you can comment on or make changes to this bug.
Description
•