Closed Bug 433168 Opened 16 years ago Closed 11 years ago

Context menu is not shown for form buttons and select elements

Categories

(Firefox :: Menus, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
Firefox 20

People

(Reporter: cjcypoi02, Assigned: johan.charlez)

References

Details

(Keywords: dev-doc-complete, regression, Whiteboard: [READ COMMENT #57])

Attachments

(2 files, 2 obsolete files)

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
Version: unspecified → Trunk
I think this is the correct behavior for most people.  Firebug just needs a way to disable it when installed.
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.
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.
(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.)
Component: Event Handling → Menus
OS: Windows XP → All
Product: Core → Firefox
QA Contact: events → menus
Hardware: PC → All
Summary: [Fx3] Context menu is not showed if you right-click on a input, button or select element → [Fx3] Context menu is not shown for button or select elements
Summary: [Fx3] Context menu is not shown for button or select elements → [Fx3] Context menu is not shown for form buttons and select elements
> 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.
(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?
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.
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.
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.
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?
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.
Status: UNCONFIRMED → NEW
Ever confirmed: true
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 ;-)
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.
Blocks: 404536
No longer depends on: 426074
(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.
Blocks: 449017
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 :(
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!
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!
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.
This happened as a result of bug 404536.
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?
(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.
(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.
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.
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.
The "Start" button and all the buttons on the taskbar have a context menu in Windows.
(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.
(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.
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.
(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?
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.
(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?
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.
Keywords: dev-doc-needed
(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
(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.
It would be very helpful to have a working sample of how to do this for developers.  Any takers to create one?
(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.)
(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?
(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.  :-)
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?
Thank you! (heads off to donate)
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...
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. :)
(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...
(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!
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.
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?
(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.
(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?
(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.
(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.
(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.
Is there any activity on this bug? It is quite important to me so I would very appreciate if somebody could fix this bug.
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
Summary: [Fx3] Context menu is not shown for form buttons and select elements → [READ COMMENT #57] Context menu is not shown for form buttons and select elements
Summary: [READ COMMENT #57] Context menu is not shown for form buttons and select elements → Context menu is not shown for form buttons and select elements
Whiteboard: [READ COMMENT #57]
Whiteboard += parity-chrome
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?
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
We need this for the "Inspect" menuitem (Devtools Inspector).

I don't believe the argument for bug 404536 applies any more.
Blocks: 724546
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?
Attached patch patch 1 (obsolete) — Splinter Review
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?
> * 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
Attachment #664645 - Flags: feedback?(mano)
(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.
Attached image 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
Assignee: nobody → johan.charlez
Status: NEW → ASSIGNED
Comment on attachment 664645 [details] [diff] [review]
patch 1

feedback+ for the trivial change, but are you asking me about the addon-issue?
Attachment #664645 - Flags: feedback?(mano) → feedback+
(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)?
Attached patch patch v2 (obsolete) — Splinter Review
* Rebased 2012-10-30
* Added comment above 'isTargetAFormControl'-method informing that it is now deprecated.
Attachment #664645 - Attachment is obsolete: true
Attachment #676753 - Flags: review?(mano)
Attachment #676753 - Flags: review?(mano) → review+
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.
(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.
Firebug is now checking existence of the isTargetAFormControl method.
https://github.com/firebug/firebug/commit/d299332052c797b6793b6ec37b65cc03e5197f11

Honza
Attached patch patch v3Splinter Review
* 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!
Attachment #676753 - Attachment is obsolete: true
Attachment #678839 - Flags: review?(mano)
Attachment #678839 - Flags: feedback?(gavin.sharp)
Attachment #678839 - Flags: feedback?(gavin.sharp) → feedback+
Review ping. This has been in review queue for over a month. Can we get someone to take a look at it please? Thanks!
Attachment #678839 - Flags: review?(mano) → review+
Can we land that?
https://hg.mozilla.org/mozilla-central/rev/bc5a2f85d15e
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 20
You need to log in before you can comment on or make changes to this bug.