Last Comment Bug 586212 - Don't carry out the command when clicking on a disabled splitmenu
: Don't carry out the command when clicking on a disabled splitmenu
Status: VERIFIED FIXED
: polish
Product: Firefox
Classification: Client Software
Component: Menus (show other bugs)
: Trunk
: x86 All
: -- normal (vote)
: Firefox 4.0
Assigned To: Dão Gottwald [:dao]
:
: Jared Wein [:jaws] (please needinfo? me)
Mentors:
http://www.einslive.de/
Depends on:
Blocks: 571782 589146
  Show dependency treegraph
 
Reported: 2010-08-11 01:38 PDT by Christian Ascheberg
Modified: 2011-03-08 04:57 PST (History)
8 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
-


Attachments
Testcase (161 bytes, text/html)
2011-02-13 11:29 PST, Christian Ascheberg
no flags Details
patch (843 bytes, patch)
2011-02-14 13:53 PST, Dão Gottwald [:dao]
dolske: review+
dolske: approval2.0+
Details | Diff | Splinter Review

Description Christian Ascheberg 2010-08-11 01:38:07 PDT
User-Agent:       Mozilla/5.0 (Windows NT 6.0; rv:2.0b4pre) Gecko/20100810 Minefield/4.0b4pre
Build Identifier: Mozilla/5.0 (Windows NT 6.0; rv:2.0b4pre) Gecko/20100810 Minefield/4.0b4pre

In javascript popups, "New Tab" in the root menu is not disabled, while in the
submenu it is disabled.
Clicking "New Tab" shows a blank page, but you can not switch tabs. You will have to close the popup.
(It is only possible to actually open a new tab, if browser.tabs.closeWindowWithLastTab is true [default])

Reproducible: Always

Steps to Reproduce:
1. Open website above (for example)
2. Click "Webradio" to open popup
3. Click the Firefox button
4. "New Tab" in the root menu is enabled, in the submenu it is disabled.
Actual Results:  
If you click it, a blank tab opens (if browser.tabs.closeWindowWithLastTab is true)
The tab is not displayed, you can not switch tabs, can not change URL.

Expected Results:  
"New Tab" in the menu root should be disabled.
Or maybe it should say "New Window".
Comment 1 Henrik Skupin (:whimboo) 2010-08-11 02:20:57 PDT
Is this a regression from the checkin of bug 571782?
Comment 2 u88484 2010-08-12 14:09:05 PDT
(In reply to comment #1)
> Is this a regression from the checkin of bug 571782?
No, in a sense this is new (not a regression in a working feature per se) since before bug 571782 landed "New Tab" wasn't available in the Firefox button.  Also, a tabbar/menu bar has never (or hasn't in forever) been shown in a popup window so this case has just never been brought to light.  I couldn't get keyboard shortcut CTRL+T to work so I'm assuming this case might have been taken care of before so maybe bug 571782 just didn't account for this.
Comment 3 u88484 2010-08-12 14:10:06 PDT
Sorry, meant to CC Dao on my last comment.  Dao, what do you think should happen here?
Comment 4 Christian Ascheberg 2010-08-12 14:45:06 PDT
In Firefox 3.6.8 the given webpage is showing a menu bar in the popup, and it is also possible to open new tabs in it, the tabbar will be displayed then.
So I think it is a rather new behaviour to disallow opening new tabs in popups.
Comment 5 Johnathan Nightingale [:johnath] 2010-08-24 08:22:23 PDT
Doesn't block, but I'd love to see a patch that made things consistent, here.
Comment 6 Christian Ascheberg 2010-08-26 03:08:28 PDT
(In reply to comment #5)
> Doesn't block, but I'd love to see a patch that made things consistent, here.

In the current build, both menu items are enabled in popups now. (I do not know what caused this.) So it is consistent, though it does not make much sense this way, as it is not possible to open new tabs in popups.
Comment 7 Christian Ascheberg 2010-08-26 03:42:13 PDT
(In reply to comment #6)
> In the current build, both menu items are enabled in popups now.

I was wrong, sorry, this is only the case when browser.tabs.closeWindowWithLastTab = false (see bug 577382). Though, in that case, it is not actually possible to open tabs (see description).
Comment 8 Christian Ascheberg 2011-02-13 11:27:41 PST
I have tested this again using the current latest-trunk version, and some things have changed. This bug is now independent from browser.tabs.closeWindowWithLastTab pref.

To test this, the popup must have the attributes menubar=1 and toolbar=0.
In that popup, the minefield button will be displayed. Clicking it will show the root menu with the item "New Tab" greyed out. The belonging submenu will not be shown on hover.

Clicking the greyed out "New Tab" item does still open a new tab in the popup though! These errors occur:

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: 5515

Trying to close the tab (seen as white content only) using CTRL+W shows this alert:

ASSERT: Giving up waiting for the tab closing animation to finish (bug 608589)
Stack Trace: 
0:([object XULElement],[object XULElement],-5)

and this error:

Error: this.selectedTab is null
Source File: chrome://browser/content/tabbrowser.xml
Line: 1679

Then closing the popup:

Error: aWindow.content is null
Source File: resource://gre/components/nsSessionStore.js
Line: 845
Comment 9 Christian Ascheberg 2011-02-13 11:29:05 PST
Created attachment 512038 [details]
Testcase
Comment 10 Dão Gottwald [:dao] 2011-02-14 13:53:56 PST
Created attachment 512267 [details] [diff] [review]
patch

fixes the remaining issue from comment 8
Comment 11 Dão Gottwald [:dao] 2011-02-14 13:58:30 PST
morphing
Comment 12 Justin Dolske [:Dolske] 2011-03-02 18:18:45 PST
Comment on attachment 512267 [details] [diff] [review]
patch

Maybe worth a 1-liner comment?

Also, we should disable the hover effect for the menu arrow part of the splitmenu when it's disabled... Followup?
Comment 13 Dão Gottwald [:dao] 2011-03-03 02:37:34 PST
http://hg.mozilla.org/mozilla-central/rev/39982a2ba344
Comment 14 Dão Gottwald [:dao] 2011-03-03 07:01:18 PST
(In reply to comment #12)
> Also, we should disable the hover effect for the menu arrow part of the
> splitmenu when it's disabled... Followup?

It's actually the other way around, the hover effect is missing for the menuitem part. Filed bug 638431.
Comment 15 Teodosia Pop 2011-03-08 04:57:51 PST
Verified using 
Mozilla/5.0 (Windows NT 6.1; rv:2.0) Gecko/20100101 Firefox/4.0 
and 
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:2.0) Gecko/20100101 Firefox/4.0.

Note You need to log in before you can comment on or make changes to this bug.