Last Comment Bug 404536 - Right click on Web page button should be ignored (don't show context menu on web form controls except textboxes)
: Right click on Web page button should be ignored (don't show context menu on ...
Status: RESOLVED FIXED
: dev-doc-complete
Product: Firefox
Classification: Client Software
Component: Menus (show other bugs)
: Trunk
: All Windows XP
: -- enhancement (vote)
: Firefox 3 beta4
Assigned To: :Ehsan Akhgari (busy, don't ask for review please)
:
Mentors:
Depends on: 424101 425631 426074 433168
Blocks:
  Show dependency treegraph
 
Reported: 2007-11-20 07:19 PST by Andrés Delfino
Modified: 2012-12-29 07:38 PST (History)
13 users (show)
ehsan: in‑testsuite+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Patch (v1) (2.28 KB, patch)
2008-01-10 06:25 PST, :Ehsan Akhgari (busy, don't ask for review please)
asaf: review+
mbeltzner: ui‑review+
Details | Diff | Review
Patch (v1.1) (2.29 KB, patch)
2008-02-22 23:24 PST, :Ehsan Akhgari (busy, don't ask for review please)
mbeltzner: approval1.9+
Details | Diff | Review
Add user pref for right click on all form elements (265 bytes, patch)
2008-06-04 10:36 PDT, Morgan 'ARR!' Allen
no flags Details | Diff | Review

Description Andrés Delfino 2007-11-20 07:19:34 PST
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
Comment 1 [:Aleksej] 2007-12-11 13:53:22 PST
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…
Comment 2 Samuel Sidler (old account; do not CC) 2007-12-11 14:03:01 PST
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...
Comment 3 Andrés Delfino 2007-12-11 14:19:21 PST
(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.
Comment 4 Samuel Sidler (old account; do not CC) 2007-12-11 14:58:32 PST
Sure. I'll confirm this as a valid RFE.

I don't see any dupes, but updating summary anyway in case someone files one.
Comment 5 :Ehsan Akhgari (busy, don't ask for review please) 2008-01-06 03:27:14 PST
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.
Comment 6 :Ehsan Akhgari (busy, don't ask for review please) 2008-01-10 06:25:18 PST
Created attachment 296318 [details] [diff] [review]
Patch (v1)

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>.
Comment 7 :Ehsan Akhgari (busy, don't ask for review please) 2008-01-10 06:26:48 PST
Targeting for Beta 3...
Comment 8 Mano (::mano, needinfo? for any questions; not reading general bugmail) 2008-01-19 05:27:12 PST
Comment on attachment 296318 [details] [diff] [review]
Patch (v1)

ui-r please.
Comment 9 :Ehsan Akhgari (busy, don't ask for review please) 2008-02-11 09:11:58 PST
Beltzner: ping.
Comment 10 :Ehsan Akhgari (busy, don't ask for review please) 2008-02-21 22:53:26 PST
Comment on attachment 296318 [details] [diff] [review]
Patch (v1)

Asking mano for review, now that this has ui-r+.
Comment 11 Mano (::mano, needinfo? for any questions; not reading general bugmail) 2008-02-22 18:19:01 PST
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.
Comment 12 :Ehsan Akhgari (busy, don't ask for review please) 2008-02-22 23:24:38 PST
Created attachment 305133 [details] [diff] [review]
Patch (v1.1)

Addressed mano's review.  Requesting approval to fix this polish issue which eliminates the menus where they are not appropriate.
Comment 13 Mike Beltzner [:beltzner, not reading bugmail] 2008-02-23 12:18:52 PST
Comment on attachment 305133 [details] [diff] [review]
Patch (v1.1)

a=beltzner
Comment 14 Reed Loden [:reed] (use needinfo?) 2008-02-23 23:42:53 PST
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
Comment 15 Andrés Delfino 2008-02-24 08:23:15 PST
Right now, the click is not ignored, instead, the control gets selected, and, in the case of buttons, it seems pressed.
Comment 16 :Ehsan Akhgari (busy, don't ask for review please) 2008-02-24 10:40:43 PST
(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?)
Comment 17 Andrés Delfino 2008-02-24 10:44:06 PST
(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.
Comment 18 Andrés Delfino 2008-02-25 07:01:11 PST
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.
Comment 19 Brian Polidoro 2008-03-19 07:15:05 PDT
Why not have a context menu for inputs where type is image so you can save the image?  
Comment 20 :Ehsan Akhgari (busy, don't ask for review please) 2008-03-19 23:50:21 PDT
(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.
Comment 21 Andrés Delfino 2008-03-20 06:02:14 PDT
(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
Comment 22 Morgan 'ARR!' Allen 2008-03-21 13:13:56 PDT
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.
Comment 23 Andrés Delfino 2008-03-21 13:35:50 PDT
(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.
Comment 24 :Gavin Sharp [email: gavin@gavinsharp.com] 2008-03-29 23:33:07 PDT
(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.
Comment 25 Andrés Delfino 2008-03-30 10:56:16 PDT
(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...
Comment 26 Kai Liu 2008-05-20 05:24:11 PDT
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.
Comment 27 Sebastian Tschan 2008-06-04 10:09:48 PDT
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.
Comment 28 Morgan 'ARR!' Allen 2008-06-04 10:35:34 PDT
I created a user preference patch when this was still bug #404536, I will add it again. 

Comment 29 Morgan 'ARR!' Allen 2008-06-04 10:36:33 PDT
Created attachment 323731 [details] [diff] [review]
Add user pref for right click on all form elements
Comment 30 :Ehsan Akhgari (busy, don't ask for review please) 2008-06-04 13:29:36 PDT
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.
Comment 31 Eric Shepherd [:sheppy] 2009-02-18 09:15:30 PST
What is the developer documentation issue here?  This looks like a user doc issue to me.
Comment 32 Brian Polidoro 2009-02-18 09:24:45 PST
See Comment #26
Comment 33 Eric Shepherd [:sheppy] 2009-02-18 09:27:42 PST
OK, but how do you override the behavior?
Comment 34 :Ehsan Akhgari (busy, don't ask for review please) 2009-02-18 09:30:58 PST
(In reply to comment #33)
> OK, but how do you override the behavior?

See bug 433168 comment 33 for an example.
Comment 35 Eric Shepherd [:sheppy] 2009-02-18 09:42:19 PST
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.
Comment 36 :Ehsan Akhgari (busy, don't ask for review please) 2009-02-18 11:54:53 PST
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>
Comment 37 Lance E Sloan 2009-02-18 12:40:58 PST
(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?
Comment 38 :Ehsan Akhgari (busy, don't ask for review please) 2009-02-18 12:43:09 PST
(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?
Comment 39 Lance E Sloan 2009-02-19 05:59:12 PST
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.
Comment 40 :Ehsan Akhgari (busy, don't ask for review please) 2009-02-19 06:52:27 PST
(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.
Comment 41 :Ehsan Akhgari (busy, don't ask for review please) 2009-03-28 23:47:13 PDT
Tests landed on mozilla-central and mozilla-1.9.1 as part of bug 424101.
Comment 42 David Balažic 2011-09-08 04:51:42 PDT
"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.
Comment 43 David Balažic 2011-09-30 05:53:52 PDT
Same in 7.0.1

I will open a new bug if this one stays marked as RESOLVED.
Comment 44 Lance E Sloan 2011-09-30 06:14:20 PDT
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.
Comment 45 Sebastian Zartner 2011-12-22 05:58:42 PST
I want to point out, that bug 433168 describes very detailed why it was a mistake to remove the context menu completely.

Sebastian
Comment 46 :Gavin Sharp [email: gavin@gavinsharp.com] 2012-12-29 07:38:24 PST
This change was reverted for Firefox 20 in bug 433168.

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