433168 - Context menu is not shown for form buttons and select elements

Status: RESOLVED FIXED

(Firefox :: Menus, defect)

Description

[Lucas Malor (mail: c6kfnkn2uc AT snkmail DOT c0m)]

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

As title, if you right-click on one of these elements, no context menu is showed. It's very irritating especially when you want to inspect them with Firebug.

Reproducible: Always




See also Bug 17754, Bug 297979 and Bug 426074

Comment 1

[Dave Garrett]

I think this is the correct behavior for most people.  Firebug just needs a way to disable it when installed.

Comment 2

[Lucas Malor (mail: c6kfnkn2uc AT snkmail DOT c0m)]

You are pretty right, but it could be useful also for Bug 17754

If developers don't want to fix Bug 17754, or they want it works as Safari (middle-click only), Bug 426074 should be fixed instead.

Comment 4

[Timwi]

Even if right-clicking the mouse produces no context menu, I argue that the Context-Menu key still should, because the keyboard event should bubble up to the content area.

If you are already a mouse user, it is easy to just right-click somewhere else, but as a keyboard user, it is quite hard to focus the content area (see Bug 146045). Given that the Context-Menu key worked fine in Firefox 2, I think functionality has degraded significantly.

Comment 6

[Kai Liu]

(In reply to comment #4)
> Even if right-clicking the mouse produces no context menu, I argue that the
> Context-Menu key still should, because the keyboard event should bubble up to
> the content area.
> 
The event is never prevented from reaching the button.  All that's going on here is the context menu is disabled for elements for which there is no context menu.

> Given that the Context-Menu key worked fine in Firefox 2, I think
> functionality has degraded significantly.
> 
By that argument, you could say because Firefox doesn't have all the preferences and features of Suite/SeaMonkey, that it is a "degraded" product.


(FWIW, I personally disagree with bug 404536 and think that it was a mistake, but the argument for its removal is nevertheless valid, even if it's not convincing to me.)

Comment 7

[Timwi]

> The event is never prevented from reaching the button.

Ah, but apparently it is prevented from reaching the content area.

> the context menu is disabled for elements for which there is no context menu.

I argue that elements which have no context menu should pass the Context-Menu key event up to their parent.

Comment 8

[Lucas Malor (mail: c6kfnkn2uc AT snkmail DOT c0m)]

(In reply to comment #6)
> The event is never prevented from reaching the button.  All that's going on
> here is the context menu is disabled for elements for which there is no 
> context menu.

There's a problem: Firebug seems to attach "Inspect element" in all context menus. So why the context menu with "Inspect element" is not shown? Firebug must mark it as it must be available also for selects and buttons in some way, or the context menu showing for selects and buttons is just disabled and stop?

Comment 10

[Dave Garrett]

The basic issue here, is that the way it is now is correct for Firefox.  It is not a Mozilla bug to debate over how it should be for Firebug.  I'll just CC John Barton here and he can decide what to do about this in Firebug.

Comment 11

[Lucas Malor (mail: c6kfnkn2uc AT snkmail DOT c0m)]

Emmh... Garrett, as I said in Comment 2 the problem is not Firebug. :-)

The problem is that if Bug 17754 is accepted, this bug must be fixed, or Bug 17754 can't be fixed. If Bug 17754 will be WONTFIXed, so this bug has no reason to be fixed. But in this case Bug 426074 must be considered. I add the dependencies.

I already filed the Firebug problem here:
http://code.google.com/p/fbug/issues/detail?id=716

I add this bug report to URL field.

Comment 12

[Dave Garrett]

Well, yes, there's other ancient never-implemented things that could use this too, but for the most part the complaints here are related to Firebug.

What I'm suggesting is that it should be possible for Firebug to overrule this when installed.

Comment 13

[Timwi]

Dave, please allow me to disagree. I do not use Firebug. (OK, I do use it, but I don't use the "Inspect Element" menu item.) However, I do use the context menu. Very frequently indeed. Things like "View Source", "Reload", or "View Page Info" cannot be accessed for a frame otherwise (the keyboard shortcuts all apply to the outer frameset, not the frame). It is quite annoying that I have to unfocus the element before I can do this.

I also don't understand the reasoning for this behaviour, which you claim is "correct"; at least not for key events (I use the Context-Key on the keyboard). Why do you think buttons and drop-downs should swallow the key event instead of passing it up to their parent?

Comment 14

[Dave Garrett]

Look, I'm not arguing that this should be that way, just that it is.  If someone decides to implement it, great.  If I was actually arguing against adding a context menu, I could've just closed this bug.  :p

(in fact, I might as well confirm this right now)

The "Inspect Element" complaint has simply popped up in here quite a few times, and _that_ part is the part I am commenting on.  That's a complaint with Firebug and not directly with Firefox.

BTW, the keyboard-only/accessibility argument is the most convincing one here, I believe.

Comment 15

[John J. Barton]

Just in case this was not clear, Firebug does not (as far as I know) do any thing special for these elements that don't show a context menu. There is a request for Firebug to create special case code for these elements so they will have a context menu (as noted above issue 716).  Obviously we'd prefer that Firefox accept the challenge ;-)

Comment 16

[Kai Liu]

The problem here is really bug 404536, which aside from causing this, also causes bug 424101.

The overall design decision in bug 404536 is correct, but I think that a different approach should be considered, one that does not cause bug 424101 (which I think is not INVA; the media tab in page info is only a workaround) and one that does not break compatibility with extensions.

I also don't think that bug 426074 is relevant here; HTML buttons should behave like XUL buttons in that right-clicking does not depress the button, and I think that's really an orthogonal issue.

Comment 17

[Lucas Malor (mail: c6kfnkn2uc AT snkmail DOT c0m)]

(In reply to comment #16)
> I also don't think that bug 426074 is relevant here [...]

You are right, I think that bug suggests to display the context menu of parent element. I'm not sure but I think a related bug with this suggestion was already filed somewhere.

Comment 18

[Des]

It's really ugly old bug and it is still NEW!!!!!
I cant use extension useful SubmitToTab and I can't inspect inputs with Firebug via context menu :(

Comment 20

[Lance E Sloan]

I will add a comment to keep this issue visible.  I upgraded from FF2 to FF3 only recently because I wanted to make sure that FF3 would be stable and support the extensions most important to me and my work.  I was very disappointed to find that contextual menus are no longer available for input buttons and select elements.  Besides making it impossible to directly inspect those elements with Firebug, this change from FF2 behavior affects other extensions, like ones for submitting forms to new tabs and so on.

Since this bug has been around since mid-2008, I would really appreciate it if some of the kindly FF developers could find a little time to just enable contextual menus for these elements.  Please?

Thank you!

Comment 21

[Henrik Skupin [:whimboo][⌚️UTC+2]]

It would be great if you could help us in finding the regression range for this bug. If someone of you have time please check the following location:

http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2008/

Enter the months and watch out for folders including the string mozilla1.9.0. Check those builds and try to bi-sec a possible date range within the regression has been started. Please make sure to not use your daily profile. Create a new one according to http://support.mozilla.com/en-US/kb/Managing+profiles. Thanks!

Comment 22

[Des]

Tried this:
firefox-3.0pre.en-US.win32.installer.exe   21-May-2008 07:57 	7.0M

Seems like there is no context menu on inputs.

Comment 23

[(no longer active)]

This happened as a result of bug 404536.

Comment 24

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Oh, you are right. So this is an expected behavior? If yes this bug is probably wontfix? Or shall we get over the attachment 323731 [details] [diff] [review] to enable it with a user pref?

Comment 25

[Lance E Sloan]

(In reply to comment #23)
> This happened as a result of bug 404536.

True.  And that seems like a bad idea to me.  The argument that contextual menus aren't available for buttons in other parts of the OS's UI is weak.  The focus of the discussion should be on the contents of the FF window, not the UI around it added by the OS.  A good example that contextual menus should be present would be Microsoft Word.  (Since the author of the counterargument used MS Windows as his example, I'll mention a MS product, too.)  When writing a document in Word, there are contextual menus available for changing the font of text, formatting tables and bullets, copy, paste, and so on.  A web page is just a document, too, so it should have contextual menus as well.

As one of the commenters on bug 404536 suggested, there should be an option to disable contextual menus.  I think that would have been the correct way to deal with this "problem".  I know that some users that are not tech savvy can get  confused (or even panic) when they right-click accidentally and get a contextual menu.  That's probably what the original requester was trying to "fix" with that earlier "bug" report.  I'm a little surprised that it was "resolved" in the way it was.

Comment 26

[(no longer active)]

(In reply to comment #24)
> Oh, you are right. So this is an expected behavior? If yes this bug is probably
> wontfix? Or shall we get over the attachment 323731 [details] [diff] [review] to enable it with a user pref?

I will be fine with enabling this by a pref, althoough it would still not be an ideal solution...  We have one side of the argument which says form buttons look like OS buttons and should act like one.  We have this other side of the argument which says they look like OS buttons but can act differently because of functionality which is only available in context menus.  A pref would be a good interim solution, but we really need to make up our minds on the fundamental issue.

We could probably use Alex's input here.

Comment 27

[John J. Barton]

Or: Firefox can decide that it won't provide any context menu entries for these elements but allow extensions to add entries for their users. The current inconsistent behavior of allowing context sometimes and not others makes users and developers unhappy.

Comment 28

[Alex Faaborg [:faaborg] (Firefox UX)]

Allowing extensions to add context menus to these controls makes sense.  In terms of matching platform behavior, by default (no extensions installed) users should not be able to get a context menu on these types of controls.

In terms of UI precedent: on on OS X and Gnome we never see context menus on these types of controls.  Windows XP will sometimes display context menus on these controls for the kind of inane "What's This?" contextual command, related to the help button in the window frame.  Although we aren't using that type of help interface, so there isn't any OS integration to expose a context menu on a button or select.

Comment 29

[Timwi]

The "Start" button and all the buttons on the taskbar have a context menu in Windows.

Comment 30

[Lance E Sloan]

(In reply to comment #28)
> Allowing extensions to add context menus to these controls makes sense.

I disagree.  Extensions may be allowed to add items to contextual menus, but I don't think they should be allowed to add or remove the menus entirely.  (For that matter, I don't like Javascript having the ability to disable menus.)

>  In terms of matching platform behavior, by default (no extensions installed) users
> should not be able to get a context menu on these types of controls.

A web page is a document, so comparing this to non-document parts of the UI provided by the OS is not so clear-cut.

> In terms of UI precedent: on on OS X and Gnome we never see context menus on
> these types of controls.  

In Mac OS X, one can right-click on the buttons at the top of Finder windows to get a contextual menu.  It contains items that let the user choose what should be shown in the the button bar and how they should appear.  This is common in many MOSX applications.

One thing to consider is that the contextual menus had been available in Firefox for years, until that decision to remove them from form elements.  There wasn't an announcement of the change, either.  Giving users the choice of disabling these menus seems like the most appropriate action.

Comment 31

[John J. Barton]

(In reply to comment #30)
> (In reply to comment #28)
> > Allowing extensions to add context menus to these controls makes sense.
> 
> I disagree.  Extensions may be allowed to add items to contextual menus, but I
> don't think they should be allowed to add or remove the menus entirely.  (For
> that matter, I don't like Javascript having the ability to disable menus.)

Why? Users have complete control over which extensions they load and they can simply not load extensions that add/remove menus.  

Firebug users complain about the missing context menu a lot. They think its a bug and report it as such.

Comment 32

[Dave Garrett]

Aside from security concerns, I don't see any particular need to restrict what can be done through extensions. Letting extensions have at this sounds best.

Comment 33

[(no longer active)]

(In reply to comment #28)
> Allowing extensions to add context menus to these controls makes sense.  In
> terms of matching platform behavior, by default (no extensions installed) users
> should not be able to get a context menu on these types of controls.

Extensions have always been able to show context menus on form controls, and add their own items if desired.  I have not tested this code, but it should work for extensions which try to bring back the old behavior of form controls having context menus:

let setTargetOriginal = nsContextMenu.setTarget;
nsContextMenu.setTarget = function(aNode, aRangeParent, aRangeOffset) {
  setTargetOriginal.apply(this, arguments);
  if (this.isTargetAFormControl(aNode))
    this.shouldDisplay = true;
}

So, given that extensions *are* able to modify the context menu behavior as desired, should we WONTFIX this bug?

Comment 34

[John J. Barton]

Since context menus are user-activated events, the proposal in comment 28 would cause every extension that needed to support context menus to run the code above on every page and on every DOM addition. That seems unreasonable and unproductive to me.  A global switch or per-window would be much better.

Comment 35

[(no longer active)]

(In reply to comment #34)
> Since context menus are user-activated events, the proposal in comment 28 would
> cause every extension that needed to support context menus to run the code
> above on every page and on every DOM addition. That seems unreasonable and
> unproductive to me.  A global switch or per-window would be much better.

If you do the functions modification on nsContextMenu's prototype, then it should be enough to do it once per window, which seems reasonable.  Or am I missing something?

Comment 36

[Eric Shepherd [:sheppy]]

There's a stubbish article here:

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

More details will be needed once this is resolved.

Comment 37

[John J. Barton]

(In reply to comment #35)
...
> If you do the functions modification on nsContextMenu's prototype, then it
> should be enough to do it once per window, which seems reasonable.  Or am I
> missing something?

I don't know what is nsContextMenu. We use 
<popup id="contentAreaContextMenu">
as described in  
https://developer.mozilla.org/en/XUL/PopupGuide/Extensions

Comment 38

[(no longer active)]

(In reply to comment #37)
> I don't know what is nsContextMenu. We use 
> <popup id="contentAreaContextMenu">
> as described in  
> https://developer.mozilla.org/en/XUL/PopupGuide/Extensions

nsContextMenu is the javascript object which corresponds to that menu <http://mxr.mozilla.org/mozilla-central/source/browser/base/content/nsContextMenu.js#61>.  With a XUL overlay adding a menuitem to the contentAreaContextMenu popup and the code in comment 33, you should get the context menu including your menu item when right clicking on form controls.

Comment 39

[Eric Shepherd [:sheppy]]

It would be very helpful to have a working sample of how to do this for developers.  Any takers to create one?

Comment 40

[(no longer active)]

(In reply to comment #39)
> It would be very helpful to have a working sample of how to do this for
> developers.  Any takers to create one?

Actually, why not create an extension which runs the code in comment 33 (possibly on nsContextMenu's prototype) so that all extensions which are affected by this change can recommend users to install that extension as well?  This way we'd kill two birds with one stone (we have a working fix for people to use immediately, and we have a real code sample for developers who don't wish to depend on that extension.)

Comment 41

[John J. Barton]

(In reply to comment #38)
...
>  With a XUL overlay adding a menuitem to the contentAreaContextMenu popup and
> the code in comment 33, you should get the context menu including your menu
> item when right clicking on form controls.

So the code in comment 33 is run once per XUL window, not once per nsIDOMWindow? That is, once in the overlay correct?

Comment 42

[(no longer active)]

(In reply to comment #41)
> So the code in comment 33 is run once per XUL window, not once per
> nsIDOMWindow? That is, once in the overlay correct?

Well, this code should match that criteria (running once per XUL window, inside the overlay):

let setTargetOriginal = nsContextMenu.prototype.setTarget;
nsContextMenu.prototype.setTarget = function(aNode, aRangeParent, aRangeOffset) {
  setTargetOriginal.apply(this, arguments);
  if (this.isTargetAFormControl(aNode))
    this.shouldDisplay = true;
}

Please note that I typed this off the top of my head, so it may not work verbatim, but it's close to what needs to be done at least.  :-)

Comment 43

[(no longer active)]

OK, I wrote that extension, it's available from <https://addons.mozilla.org/addon/10801/>

The extension is quite simple, and uses the same code I posted in comment 42.  Hope it's useful for you guys.

BTW, Eric, maybe you can add this extension as a workaround to the MDC article if needed?

Comment 44

[Ted Gifford]

Thank you! (heads off to donate)

Comment 45

[Boris Zbarsky [:bzbarsky]]

Wouldn't the right approach to extensions be to not show the context menu if it has no options?  Then put no Firefox options in the context menu for inputs if that's desired.  If an extension adds options, the menu will appear without the extension having to jump through hoops...

Comment 46

[Eric Shepherd [:sheppy]]

Ehsan -- you are my hero.  The article here:

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

Now takes advantage of your code to describe this much better.  Please feel free to tweak my text if I don't make any sense. :)

Comment 47

[(no longer active)]

(In reply to comment #45)
> Wouldn't the right approach to extensions be to not show the context menu if it
> has no options?  Then put no Firefox options in the context menu for inputs if
> that's desired.  If an extension adds options, the menu will appear without the
> extension having to jump through hoops...

Hmm, you mean emptying the menu on form controls by default, allowing extensions to add stuff to them as desired, and actually showing the menu if it's not empty?

That would be a whole different approach to the current way in which the content area context menu works.  Currently, the menu includes the union of all the items which will ever get displayed in the menu, and on right-click, based on the active element, the undesired elements will be hidden.

Also, we would still need another work around for stuff like bug 424101...

Comment 48

[(no longer active)]

(In reply to comment #46)
> Ehsan -- you are my hero.  The article here:
> 
> https://developer.mozilla.org/En/Offering_a_context_menu_for_form_controls
> 
> Now takes advantage of your code to describe this much better.  Please feel
> free to tweak my text if I don't make any sense. :)

The text looks perfect.  Thanks for writing it up!

Comment 49

[Boris Zbarsky [:bzbarsky]]

Yeah, you'd just mark everything you know about as hidden, then check whether that left anything.

I'm really not sure what the problem in bug 424101 is other than what looks to me like incorrectly lumping the image in with other controls.

Comment 50

[John J. Barton]

What happens if more than one extension calls the code in comment 42?  I guess the context menu runs a longer chain of redundant calls?  Is it a problem?

Comment 51

[(no longer active)]

(In reply to comment #50)
> What happens if more than one extension calls the code in comment 42?  I guess
> the context menu runs a longer chain of redundant calls?

Yes.

>  Is it a problem?

No, AFAICT.  Of course it will be a little less efficient, but it shouldn't be a big deal.

Comment 52

[John J. Barton]

(In reply to comment #38)
> nsContextMenu is the javascript object which corresponds to that menu
> <http://mxr.mozilla.org/mozilla-central/source/browser/base/content/nsContextMenu.js#61>.
>  With a XUL overlay adding a menuitem to the contentAreaContextMenu popup and
> the code in comment 33, you should get the context menu including your menu
> item when right clicking on form controls.

I still don't get it.  Is nsContextMenu a XUL window global property?

Comment 53

[John J. Barton]

(In reply to comment #11)
> I already filed the Firebug problem here:
> http://code.google.com/p/fbug/issues/detail?id=716

I applied the code in comment 33 to Firebug at R2017 on branches/firebug1.4; this fix will be in Firebug 1.4a13.

Thanks for all of the help.

Comment 54

[(no longer active)]

(In reply to comment #52)
> I still don't get it.  Is nsContextMenu a XUL window global property?

It's a global object declared at the scope of the XUL window.

Comment 55

[Martin Katz, Ph.D.]

(In reply to comment #25 and several others)
> The argument that contextual menus aren't available for buttons in 
> other parts of the OS's UI is weak.

Not just weak, but inane. Mozilla software should not emulate the bad features of any other product, and most appearance issues should be part of the "skin" or style sheet.

Since important object-specific features cannot be in the main menu, they should be in the context menu. The context menu item "open in new tab" on a form submit button should be available in the default configuration. 

Remember, as far as a normal user is concerned, the "Submit" button is the same as an IMG link. The special role in a form doesn't change that.

Comment 56

[stefanxe]

Is there any activity on this bug? It is quite important to me so I would very appreciate if somebody could fix this bug.

Comment 57

[Lucas Malor (mail: c6kfnkn2uc AT snkmail DOT c0m)]

Workarounds are:
 - update Firebug to version 1.4 or higher (if you need only it), OR
 - see https://developer.mozilla.org/En/Offering_a_context_menu_for_form_controls
(thanks to Ehsan Akhgari and Eric Shepherd)

I want also to remember that this bug is stricty related to Bug 17754. See also Comment #17

Comment 61

[Full Decent]

Whiteboard += parity-chrome

Comment 63

[Geoff Smith]

I was directed here when the problem I reported 624735 was marked as a duplicate.

I wanted to add the following information that I want to use the right click context menu for the AutoFill forms extension which is another case in addition to Firebug.

With FF4.0b8 I get this problem on my home PC with Windows Vista 32 bit, however on my work PC with Windows 7 64bit I DO get a context menu on a right click.

Also previous to using FF4.0b8 I was also getting a right click context menu on these elements with FF3.6, i was using the AutoFill extension sucessfully, but now reverting to 3.6 also shows this problem.

What could the difference be between my home and work machine that produces the difference in behaviour?

Comment 65

[Sebastian Zartner [:sebo]]

I am really surprised, that bug 404536 got implemented.
The argument to remove the context menu was pretty weak. So in that sense I agree to comment #25.

Though the simple question here is: What's the purpose of a context menu? The answer: To get contextual options for the clicked element. In the case of form elements there really might not be that many possibilities. So it might be legit to say, checkboxes and radio buttons don't need a context menu. Though the usability suffers from this a lot. I guess that's also the reason why all three other big browsers out there offer the standard page context menu on form elements. Also a selectbox might offer options like "Select All".
And as soon as an extension overlays the context menu, it should definitely be shown.

There's currently a new issue in Firebug related to that:
http://code.google.com/p/fbug/issues/detail?id=5065

And also the integrated webdev tools will need the context menu to show the inspect option.

Sebastian

Comment 66

[Johan C]

We need this for the "Inspect" menuitem (Devtools Inspector).

I don't believe the argument for bug 404536 applies any more.

Comment 67

[Roger Lynn]

I don't believe the argument for bug 404536 ever applied. Why should right clicking on a submit link give a different result to clicking on any other link? It's completely inconsistent and significantly reduces the available functionality. In particular it prevents opening the linked page in a new tab or window or saving the linked page. Why would we want to do that?

Comment 68

[Johan C]

Attachment: patch 1

This essentially removes the code bug 404536 added.

Being able to right click form elements would be great to have for devtools (Inspect).

This patch should also solve bug 794120 (no exceptions thrown with Firebug installed).

Furthermore, no other browser (on Windows 7) ignores right click, doesn't make sense for Firefox to do so.

I hope this patch can start an active discussion on this issue. For now I'm not sure whom to CC or flag for feedback.

Patch notes:
* I have left the 'isTargetAFormControl'-method in to avoid breaking Firebug, and possibly other extensions?

Comment 69

[Sebastian Zartner [:sebo]]

> * I have left the 'isTargetAFormControl'-method in to avoid breaking Firebug, and 
> possibly other extensions?
For us (the Firebug Working Group) it wouldn't be a problem to remove our workaround as soon as this got implemented. So for us it would be fine if the isTargetAFormControl() got removed. Though other extensions might have problems.

Is there a possibility for the AMO people to find out which extensions are using this function?

Sebastian

Comment 70

[Johan C]

Some extensions calling said method that I found (There could be many more on AMO though):

1. https://addons.mozilla.org/en-US/firefox/addon/form-control-context-menu/
  * https://addons.mozilla.org/en-US/firefox/files/browse/47295/file/chrome/formcontrolcontextmenu.jar/content/overlay.js#L43

2. http://code.google.com/p/reply-manager/
  * http://code.google.com/p/reply-manager/source/browse/suite/common/nsContextMenu.js?r=b28c21c3896ff65a451f833498df84caf760929e#476
  * http://code.google.com/p/reply-manager/source/browse/suite/common/nsContextMenu.js?r=b28c21c3896ff65a451f833498df84caf760929e#1361

Comment 71

[(no longer active)]

(In reply to comment #70)
> Some extensions calling said method that I found (There could be many more on
> AMO though):
> 
> 1. https://addons.mozilla.org/en-US/firefox/addon/form-control-context-menu/
>   *
> https://addons.mozilla.org/en-US/firefox/files/browse/47295/file/chrome/formcontrolcontextmenu.jar/content/overlay.js#L43

The only reason I wrote that extension was to enable the context menu on form controls again, so that can be ignored.

Comment 72

[Johan C]

Attachment: bug #1

(In reply to Ehsan Akhgari [:ehsan] from comment #71)
> (In reply to comment #70)
> 
> The only reason I wrote that extension was to enable the context menu on
> form controls again, so that can be ignored.

Unfortunately with your extension installed and the method gone, the following is "thrown" in the error console when launching Firefox:
http://pastebin.mozilla.org/1844326

> For us (the Firebug Working Group) it wouldn't be a problem to remove our
> workaround as soon as this got implemented. So for us it would be fine if
> the isTargetAFormControl() got removed. Though other extensions might have
> problems.

Also, if either Firebug or Ehsan's extension is installed and the method removed, "all?" possible menuitems in the context menu are added (see screenshot 'bug #1') and the following exceptions are thrown:

1. When right clicking anywhere on the page (not in the chrome):
  Timestamp: 2012-09-26 22:15:34
  Error: TypeError: this.isTargetAFormControl is not a function
  Source File: chrome://formcontrolcontextmenu/content/overlay.js
  Line: 43

2. After right clicking and performing any action that will close the context menu. (e.g. alt-tab, click page/chrome
  Timestamp: 2012-09-26 22:15:35
  Error: TypeError: gContextMenu is null
  Source File: chrome://browser/content/browser.xul
  Line: 1

Comment 73

[Asaf Romano (gone)]

Comment on attachment 664645 [details] [diff] [review]
patch 1

feedback+ for the trivial change, but are you asking me about the addon-issue?

Comment 74

[Johan C]

(In reply to Mano from comment #73)
> Comment on attachment 664645 [details] [diff] [review]
> patch 1
> 
> feedback+ for the trivial change, but are you asking me about the
> addon-issue?

I'm not sure what the best practise in this situation is.

Should we remove the method as well and notify add-on authors of this change?

Or should we leave the method alone for one version (i.e. if this patch lands in fx18, remove the method in fx19)?

Comment 75

[Johan C]

Attachment: patch v2

* Rebased 2012-10-30
* Added comment above 'isTargetAFormControl'-method informing that it is now deprecated.

Comment 76

[(no longer active)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/f3bb2259ca27

Comment 77

[Ed Morley [:emorley]]

Backed out for browser-chrome failures:
https://tbpl.mozilla.org/?tree=Mozilla-Inbound&rev=f3bb2259ca27

https://hg.mozilla.org/integration/mozilla-inbound/rev/d60e9531858f

Comment 78

[:Gavin Sharp [email: [e-mail]]]

Comment on attachment 676753 [details] [diff] [review]
patch v2

>diff --git a/browser/base/content/nsContextMenu.js b/browser/base/content/nsContextMenu.js

>+  // This method has been deprecated. See bug 433168 for more information.

>   isTargetAFormControl: function(aNode) {

I think we should just remove it - the only addons that use it appear to be Firebug (which we can get fixed pretty easily by CCing the developers here), Ehsan's add-on, and maybe one or two more. Not worth keeping around just for those.

Comment 79

[(no longer active)]

(In reply to comment #78)
> Comment on attachment 676753 [details] [diff] [review]
>   --> https://bugzilla.mozilla.org/attachment.cgi?id=676753
> patch v2
> 
> >diff --git a/browser/base/content/nsContextMenu.js b/browser/base/content/nsContextMenu.js
> 
> >+  // This method has been deprecated. See bug 433168 for more information.
> 
> >   isTargetAFormControl: function(aNode) {
> 
> I think we should just remove it - the only addons that use it appear to be
> Firebug (which we can get fixed pretty easily by CCing the developers here),
> Ehsan's add-on, and maybe one or two more. Not worth keeping around just for
> those.

I agree.

Comment 80

[Jan Honza Odvarko [:Honza] (always need-info? me)]

Firebug is now checking existence of the isTargetAFormControl method.
https://github.com/firebug/firebug/commit/d299332052c797b6793b6ec37b65cc03e5197f11

Honza

Comment 81

[Johan C]

Attachment: patch v3

* rebased
* removed the 'isTargetAFormControl'-method
* fixed test breakage.

http://mxr.mozilla.org/mozilla-central/source/browser/base/content/test/browser_bug424101.js
Is this test still needed? It was added by bug 424101.

...or is the test something we always wanted for context menus?
I have updated the test to fix the test breakage for now.

Cheers!

Comment 82

[Rob Campbell [:rc] (:robcee)]

Review ping. This has been in review queue for over a month. Can we get someone to take a look at it please? Thanks!

Comment 83

[Paul Rouget [:paul]]

Can we land that?

Comment 84

[:Gavin Sharp [email: [e-mail]]]

Yes.

Comment 85

[(no longer active)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/bc5a2f85d15e

Comment 86

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/bc5a2f85d15e