Closed Bug 404536 Opened 17 years ago Closed 16 years ago

Right click on Web page button should be ignored (don't show context menu on web form controls except textboxes)

Categories

(Firefox :: Menus, enhancement)

All
Windows XP
enhancement
Not set
normal

Tracking

()

RESOLVED FIXED
Firefox 3 beta4

People

(Reporter: adelfino, Assigned: ehsan.akhgari)

References

Details

(Keywords: dev-doc-complete)

Attachments

(2 files, 1 obsolete file)

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9b2pre) Gecko/2007112003 Minefield/3.0b2pre
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9b2pre) Gecko/2007112003 Minefield/3.0b2pre

Right clicking on a Web page button should be ignored; since that happens when you right click any "real" button, at least, in Windows (like Close in title bar). Often, buttons have a "What is this?" menu, but that does not relate to a Web page.

Reproducible: Always
Version: unspecified → Trunk
In GNOME with metacity here:
— a normal button gets focussed;
— a titlebar button shows the menu that appears anywhere else on the titlebar;
— a web page button in Firefox shows both things.

Maybe a right‐click of a web page button could do something more useful…
In Internet Explorer 7 on Windows XP, right clicking on a button yields a context menu that is much shorter than our but has everything disabled. I'm not sure why we'd change this...
Component: General → Menus
QA Contact: general → menus
(In reply to comment #2)
> In Internet Explorer 7 on Windows XP, right clicking on a button yields a
> context menu that is much shorter than our but has everything disabled. I'm not
> sure why we'd change this...
> 

Because IMHO forms should act as native (as in non Web) forms.
Sure. I'll confirm this as a valid RFE.

I don't see any dupes, but updating summary anyway in case someone files one.
Severity: normal → enhancement
Status: UNCONFIRMED → NEW
Ever confirmed: true
Summary: Right click on Web page button should be ignored → Right click on Web page button should be ignored (don't show context menu on web form buttons)
In native forms, other types of controls such as check boxes, radio buttons, combo boxes, etc. don't have context menus, right?

So I suppose lack of context menu is what one would expect for all types of form elements, except text boxes (<input type=text>, <input type=password>, <textarea>).

Editing the summary accordingly.
Summary: Right click on Web page button should be ignored (don't show context menu on web form buttons) → Right click on Web page button should be ignored (don't show context menu on web form controls except textboxes)
OS: Windows XP → All
Hardware: PC → All
Attached patch Patch (v1) (obsolete) — Splinter Review
This patch disabled context menu on all form elements, except the following:

 * <input type="text">
 * <input type="password">
 * <textarea>

All of such controls are present on <https://bugzilla.mozilla.org/enter_bug.cgi?product=Firefox> for testing purposes.  In order to test <optgroup>, you can use <http://www.w3schools.com/tags/tag_optgroup.asp>.
Assignee: nobody → ehsan.akhgari
Status: NEW → ASSIGNED
Attachment #296318 - Flags: review?(mano)
Targeting for Beta 3...
Target Milestone: --- → Firefox 3 M11
Comment on attachment 296318 [details] [diff] [review]
Patch (v1)

ui-r please.
Attachment #296318 - Flags: review?(mano)
Attachment #296318 - Flags: ui-review?(beltzner)
Whiteboard: [has patch] [needs ui-review beltzner]
Beltzner: ping.
Target Milestone: Firefox 3 beta3 → Firefox 3 beta4
Attachment #296318 - Flags: ui-review?(beltzner) → ui-review+
Comment on attachment 296318 [details] [diff] [review]
Patch (v1)

Asking mano for review, now that this has ui-r+.
Attachment #296318 - Flags: review?(mano)
Whiteboard: [has patch] [needs ui-review beltzner] → [has patch] [needs review mano]
Comment on attachment 296318 [details] [diff] [review]
Patch (v1)

>Index: browser/base/content/nsContextMenu.js
>===================================================================
>RCS file: /cvsroot/mozilla/browser/base/content/nsContextMenu.js,v
>retrieving revision 1.31
>diff -u -8 -p -r1.31 nsContextMenu.js
>--- browser/base/content/nsContextMenu.js	19 Dec 2007 17:25:09 -0000	1.31
>+++ browser/base/content/nsContextMenu.js	10 Jan 2008 14:22:15 -0000
>@@ -348,17 +348,18 @@ nsContextMenu.prototype = {
>   initMetadataItems: function() {
>     // Show if user clicked on something which has metadata.
>     this.showItem("context-metadata", this.onMetaDataItem);
>   },
> 
>   // Set various context menu attributes based on the state of the world.
>   setTarget: function (aNode, aRangeParent, aRangeOffset) {
>     const xulNS = "http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul";
>-    if (aNode.namespaceURI == xulNS) {
>+    if (aNode.namespaceURI == xulNS ||
>+        this.isTargetAFormControl(aNode)) {
>       this.shouldDisplay = false;
>       return;
>     }
> 
>     // Initialize contextual info.
>     this.onImage           = false;
>     this.onLoadedImage     = false;
>     this.onStandaloneImage = false;
>@@ -1065,16 +1066,28 @@ nsContextMenu.prototype = {
>     return "contextMenu.target     = " + this.target + "\n" +
>            "contextMenu.onImage    = " + this.onImage + "\n" +
>            "contextMenu.onLink     = " + this.onLink + "\n" +
>            "contextMenu.link       = " + this.link + "\n" +
>            "contextMenu.inFrame    = " + this.inFrame + "\n" +
>            "contextMenu.hasBGImage = " + this.hasBGImage + "\n";
>   },
> 
>+  // Returns true if node is a from control (except text boxes).
>+  // This is used to disable the context menu for form controls.
>+  isTargetAFormControl: function(node) {

aNode (in the comment too)/

r=mano otherwise.
Attachment #296318 - Flags: review?(mano) → review+
Attached patch Patch (v1.1)Splinter Review
Addressed mano's review.  Requesting approval to fix this polish issue which eliminates the menus where they are not appropriate.
Attachment #296318 - Attachment is obsolete: true
Attachment #305133 - Flags: approval1.9?
Whiteboard: [has patch] [needs review mano] → [has patch] [has review] [has ui-review] [needs approval]
Comment on attachment 305133 [details] [diff] [review]
Patch (v1.1)

a=beltzner
Attachment #305133 - Flags: approval1.9? → approval1.9+
Keywords: checkin-needed
Whiteboard: [has patch] [has review] [has ui-review] [needs approval]
Checking in browser/base/content/nsContextMenu.js;
/cvsroot/mozilla/browser/base/content/nsContextMenu.js,v  <--  nsContextMenu.js
new revision: 1.37; previous revision: 1.36
done
Status: ASSIGNED → RESOLVED
Closed: 16 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Right now, the click is not ignored, instead, the control gets selected, and, in the case of buttons, it seems pressed.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
(In reply to comment #15)
> Right now, the click is not ignored, instead, the control gets selected, and,
> in the case of buttons, it seems pressed.

Andrés: isn't this bug filed about not showing the context menu (judging from the summary?)
(In reply to comment #16)
> (In reply to comment #15)
> > Right now, the click is not ignored, instead, the control gets selected, and,
> > in the case of buttons, it seems pressed.
> 
> Andrés: isn't this bug filed about not showing the context menu (judging from
> the summary?)
> 

I didn't add the last part of the summary. The bug is about ignoring the secondary click on form controls. As the part that I did write in the summary, and comment 0 suggest.
Changing OS to Windows XP, since focusing a control when right clicking it is correct in GTK+ platforms.

In Windows XP, it is not. Don't have Windows Vista to try.
OS: All → Windows XP
Why not have a context menu for inputs where type is image so you can save the image?  
(In reply to comment #19)
> Why not have a context menu for inputs where type is image so you can save the
> image?  

This is indeed a valid issue.  Would you file a new bug, mark it blocking this bug, and CC me on that?  I'd be willing to provide the patch.
(In reply to comment #20)
> This is indeed a valid issue.  Would you file a new bug, mark it blocking this
> bug, and CC me on that?  I'd be willing to provide the patch.

Since I'm kind of responsible, I did the job: bug 424101
What about in a case where someone wished to use say firebugs 'Inspect Element'? It is a bit of a pain to have to click a nearby element, then search through the tree or use firebugs Inspect button (especially if console is closed). Just a thought.
(In reply to comment #22)
> What about in a case where someone wished to use say firebugs 'Inspect
> Element'? It is a bit of a pain to have to click a nearby element, then search
> through the tree or use firebugs Inspect button (especially if console is
> closed). Just a thought.
> 

What about it? I don't see how it wouldn't work. It does work with paragraphs, which are not form controls.
(In reply to comment #15)
> Right now, the click is not ignored, instead, the control gets selected, and,
> in the case of buttons, it seems pressed.

Even if this bug got morphed from the original intent, reopening it for additional fixes just causes trouble and makes it hard to keep track of what change was made when. Please file a new bug on actually ignoring the clicks for form controls.
Status: REOPENED → RESOLVED
Closed: 16 years ago16 years ago
Resolution: --- → FIXED
(In reply to comment #24)
> (In reply to comment #15)
> > Right now, the click is not ignored, instead, the control gets selected, and,
> > in the case of buttons, it seems pressed.
> 
> Even if this bug got morphed from the original intent, reopening it for
> additional fixes just causes trouble and makes it hard to keep track of what
> change was made when. Please file a new bug on actually ignoring the clicks for
> form controls.
> 

Don't know how can reopening a bug cause any trouble, but, whatever: bug 426074
Same description...
Depends on: 426074
This change should probably be documented somewhere, so that extensions that make use of these context menus (Firebug, SubmitToTab, et al.) will know that they need to override this new behavior.
Keywords: dev-doc-needed
Is there already documentation available how to override this behavior?
I confirm what Kai Liu has pointed out: There are several extensions (I'll add Autofill Forms to the list) which make use of the context menu for form controls and have somehow lost some functionality with this change.

In my opinion the context menu on form controls has never been a bug but a feature. I don't see any enhancement at all by disabling it.
There should at least be a user preference to re-enable the context menu for all form controls.
I created a user preference patch when this was still bug #404536, I will add it again. 

Morgan,

This bug has been marked as RESOLVED FIXED, so a new patch on it won't get noticed.  I suggest you file a new bug for controlling this behavior as a pref, post your patch on that bug, and request review there.  Also, please try to create a unified patch, as that's the format most reviewers are comfortable with.  See <http://developer.mozilla.org/en/docs/Creating_a_patch> for more details.
Depends on: 433168
What is the developer documentation issue here?  This looks like a user doc issue to me.
OK, but how do you override the behavior?
(In reply to comment #33)
> OK, but how do you override the behavior?

See bug 433168 comment 33 for an example.
OK, I've added this article:

https://developer.mozilla.org/En/Offering_a_context_menu_for_form_controls

And https://developer.mozilla.org/En/Notable_bugs_fixed_in_Firefox_3 has been
updated to mention this issue in Firefox 3.  Marking this as doc-complete, in
favor of bug 433168, which covers the workaround.
For those users who were annoyed by this fix, I wrote an extension named Form Control Context Menu which restores the old behavior of showing the context menu on all elements.

You can install the extension here: <https://addons.mozilla.org/firefox/10801>
(In reply to comment #36)
> For those users who were annoyed by this fix, I wrote an extension named Form
> Control Context Menu which restores the old behavior of showing the context
> menu on all elements.

Thanks for jumping into the fray to offer a fix to this "fix".  I notice it doesn't work with disabled form buttons, though.  Would it be possible to allow that?
(In reply to comment #37)
> Thanks for jumping into the fray to offer a fix to this "fix".  I notice it
> doesn't work with disabled form buttons, though.  Would it be possible to allow
> that?

Weird, since my code doesn't care whether the element is disabled or not... Can you please provide a test page which I can use to figure what's going on?
Depends on: 424101
Yes!  Actually, I saw the behavior here before I logged in:

  <https://addons.mozilla.org/firefox/10801>

When not logged in, the textbox for entering a review and the "Save" button below it are disabled.  A contextual menu won't appear for them.
(In reply to comment #39)
> Yes!  Actually, I saw the behavior here before I logged in:
> 
>   <https://addons.mozilla.org/firefox/10801>
> 
> When not logged in, the textbox for entering a review and the "Save" button
> below it are disabled.  A contextual menu won't appear for them.

Well, this is not related to this change.  To make sure, I tested it with Firefox 2 and it also doesn't show any context menus on the disabled form controls.
Tests landed on mozilla-central and mozilla-1.9.1 as part of bug 424101.
Flags: in-testsuite+
"Right click on Web page button should be ignored"
This is not fixed. Right click on a submit button (like the "Save Changes" on this page) makes it depressed. As if the left mouse button would be held.

Firefox 6.0.2 on Windows XP.
Same in 7.0.1

I will open a new bug if this one stays marked as RESOLVED.
I consider this bug report to be wrong.  Right click on a button should open a contextual menu with appropriate choices.  For example, submitting to another window or tab would be good.  I know there are extensions to enable this behavior, but it should be standard behavior in Firefox.
I want to point out, that bug 433168 describes very detailed why it was a mistake to remove the context menu completely.

Sebastian
This change was reverted for Firefox 20 in bug 433168.
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: