Don't carry out the command when clicking on a disabled splitmenu

VERIFIED FIXED in Firefox 4.0

Status

()

Firefox
Menus
VERIFIED FIXED
7 years ago
6 years ago

People

(Reporter: Christian Ascheberg, Assigned: dao)

Tracking

({polish})

Trunk
Firefox 4.0
x86
All
polish
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(blocking2.0 -)

Details

(URL)

Attachments

(2 attachments)

(Reporter)

Description

7 years ago
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".
(Reporter)

Updated

7 years ago
Blocks: 571782
Version: unspecified → Trunk
Is this a regression from the checkin of bug 571782?
Status: UNCONFIRMED → NEW
blocking2.0: --- → ?
Ever confirmed: true

Comment 2

7 years ago
(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

7 years ago
Sorry, meant to CC Dao on my last comment.  Dao, what do you think should happen here?
(Reporter)

Comment 4

7 years ago
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.
Doesn't block, but I'd love to see a patch that made things consistent, here.
blocking2.0: ? → -
Keywords: polish
(Reporter)

Comment 6

7 years ago
(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.
(Reporter)

Comment 7

7 years ago
(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).
(Assignee)

Updated

7 years ago
Assignee: nobody → dao
(Reporter)

Comment 8

6 years ago
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
(Reporter)

Comment 9

6 years ago
Created attachment 512038 [details]
Testcase
(Assignee)

Comment 10

6 years ago
Created attachment 512267 [details] [diff] [review]
patch

fixes the remaining issue from comment 8
Attachment #512267 - Flags: review?(dolske)
(Assignee)

Comment 11

6 years ago
morphing
Blocks: 589146
Summary: "New Tab" not disabled in popups → Don't carry out the command when clicking on a disabled splitmenu
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?
Attachment #512267 - Flags: review?(dolske)
Attachment #512267 - Flags: review+
Attachment #512267 - Flags: approval2.0+
(Assignee)

Comment 13

6 years ago
http://hg.mozilla.org/mozilla-central/rev/39982a2ba344
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 4.0
(Assignee)

Comment 14

6 years ago
(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

6 years ago
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.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.