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

Status: RESOLVED FIXED

(Firefox :: Menus, enhancement)

Description

[Andrés Delfino]

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]]

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)]

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]

(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)]

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

[(no longer active)]

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

[(no longer active)]

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

[(no longer active)]

Targeting for Beta 3...

Comment 8

[Asaf Romano (gone)]

Comment on attachment 296318 [details] [diff] [review]
Patch (v1)

ui-r please.

Comment 9

[(no longer active)]

Beltzner: ping.

Comment 10

[(no longer active)]

Comment on attachment 296318 [details] [diff] [review]
Patch (v1)

Asking mano for review, now that this has ui-r+.

Comment 11

[Asaf Romano (gone)]

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

[(no longer active)]

Attachment: 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]]

Comment on attachment 305133 [details] [diff] [review]
Patch (v1.1)

a=beltzner

Comment 14

[Reed Loden [:reed]]

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]

Right now, the click is not ignored, instead, the control gets selected, and, in the case of buttons, it seems pressed.

Comment 16

[(no longer active)]

(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]

(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]

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]

Why not have a context menu for inputs where type is image so you can save the image?  

Comment 20

[(no longer active)]

(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]

(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]

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]

(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: [e-mail]]]

(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]

(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]

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]

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]

I created a user preference patch when this was still bug #404536, I will add it again. 

Comment 29

[Morgan 'ARR!' Allen]

Attachment: Add user pref for right click on all form elements

Comment 30

[(no longer active)]

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]]

What is the developer documentation issue here?  This looks like a user doc issue to me.

Comment 32

[Brian Polidoro]

See Comment #26

Comment 33

[Eric Shepherd [:sheppy]]

OK, but how do you override the behavior?

Comment 34

[(no longer active)]

(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]]

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

[(no longer active)]

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]

(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

[(no longer active)]

(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]

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

[(no longer active)]

(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

[(no longer active)]

Tests landed on mozilla-central and mozilla-1.9.1 as part of bug 424101.

Comment 42

[David Balažic]

"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]

Same in 7.0.1

I will open a new bug if this one stays marked as RESOLVED.

Comment 44

[Lance E Sloan]

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 [:sebo]]

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: [e-mail]]]

This change was reverted for Firefox 20 in bug 433168.