Closed Bug 626326 Opened 11 years ago Closed 9 years ago

add-on bar should not show context menu when a descendent node was right-click target

Categories

(Add-on SDK Graveyard :: General, defect, P2)

defect

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: dietrich, Assigned: dietrich)

References

Details

(Whiteboard: [addon bar])

Attachments

(3 files)

add-ons often have context menus. currently, if you right click an item in the add-on bar, both the add-on context menu and the add-on bar content menu are shown.
Whiteboard: [addon bar]
Requesting blocking. Add-on context menus are common.
blocking2.0: --- → ?
Summary: add-on bar should not show context menu when a child node was right-click target → add-on bar should not show context menu when a descendent node was right-click target
This is wfm, in linux with abp, noscript and firebug; windows only?
blocking2.0: ? → final+
Whiteboard: [addon bar] → [addon bar][softblocker]
WFM with Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0b10pre) Gecko/20110125 Firefox/4.0b10pre and ABP.
Can we get a Mac check?
WFM with Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:2.0b10pre) Gecko/20110125 Firefox/4.0b10pre and ABP.

.. though it's possible that ABP could be doing something special here with event.stopPropagation() etc.
If I'm reading this bug correctly it is not fixed. When I context click on the amazing Hard Blocker Counter extension button in the add-on bar I get the add-on menu and the toolbar context menu. See http://grab.by/8zz7 which is today's Windows nightly build.
Hm, interesting. I think the problem is slightly different than I put in the summary originally.

Looks like Widgets from the Add-on SDK are responding to right-click as if it was a regular click. -> moving to Add-on SDK.
Component: Toolbars → General
Product: Firefox → Add-on SDK
QA Contact: toolbars → general
Version: Trunk → unspecified
Whiteboard: [addon bar][softblocker] → [addon bar]
Myk, this seems like a pretty bad behavior for any add-on that relies on context clicks. Is there a way for me to nominate this for a nearby release?
Asa: Add-on SDK beta releases are time-based, so this wouldn't block one, but I'd certainly love to see this problem be fixed in the next beta, which ships later this month.  Setting the target milestone accordingly.

Dietrich: is your time still entirely taken up by Firefox 4 blocker bugs, or might you have a bit of time to look into this?
Target Milestone: --- → 1.0b3
I don't know if disabling right-click in widget-space is the right thing to do long term, but it'll do for now.

The workaround for add-ons that need right-click support is to handle it in a content script and msg back to their privileged code.
--> Moving to Firefox to get rid of the blocking flag; this is an SDK bug.
Product: Add-on SDK → Firefox
QA Contact: general → general
Target Milestone: 1.0b3 → ---
--> Moving back.
blocking2.0: final+ → ---
Product: Firefox → Add-on SDK
QA Contact: general → general
Attachment #510125 - Flags: review?(adw)
Assignee: nobody → dietrich
Target Milestone: --- → 1.0b3
Comment on attachment 510125 [details]
Pointer to pull request

If I'm reading this bug right, there are two problems: 1) the widget "click" event is fired for right-clicks, but it should only be fired for left-clicks (comment 8), and 2) the add-on bar's context menu is shown when you right-click a widget (comment 7).

Is that right?  This patch fixes problem 1 but not problem 2, right?

(In reply to comment #12)
> I don't know if disabling right-click in widget-space is the right thing to do
> long term, but it'll do for now.

I think the widget "click" event should be for left-clicks only, so this patch is fine with me.  Eventually we should add a "RightClick" event and a nice API for attaching context menus to widgets similar to the context-menu API.

I made some inline comments on GitHub.  r+ with the changes mentioned there, but problem 2, if it is in fact happening, needs to be fixed too.
Attachment #510125 - Flags: review?(adw) → review+
Yep, will fix comment 7 in this bug too.
(In reply to comment #16)
> Yep, will fix comment 7 in this bug too.

Too hasty! Current policy in Firefox is to show the host toolbar's context menu for child items that do not have a context menu of their own.
latest attachment fixes adw's comments, but not the add-on bar context menu bit.
Attachment #515930 - Flags: review?(adw)
Hm, this is a right patch on the wrong bug.

The patch makes widgets stop treating right-clicks as left-clicks.

The patch does not resolve the issue in the summary, and comment #7.
Comment on attachment 515930 [details]
Pointer to pull request

This seems to be the same pull request I already r+'ed.
Attachment #515930 - Flags: review?(adw)
(In reply to comment #21)
> Comment on attachment 515930 [details]
> Pointer to pull request
> 
> This seems to be the same pull request I already r+'ed.

The pull request URI is always the same, even after additional commits, if that's what you mean.

However, this bug is not done. I need to look at how Hard Blocker Counter is providing a context menu, and if possible, not show the toolbar context menu when the add-on's context menu is shown.
I'm splitting this patch off into bug 642166, so we can land that, while fixing the toolbar context menu part here, without things getting more muddy.
Priority: -- → P3
Target Milestone: 1.0b3 → 1.0
(automatic reprioritization of 1.0 bugs)
Priority: P3 → P2
Target Milestone: 1.0 → 1.1
With my add-on Canadian Weather, I display a panel on right-click. Works properly with Windows 7, but Ubuntu 11.04 will also display the toolbar context menu. Firefox 5.0
(Pushing all open bugs to the --- milestone for the new triage system)
Target Milestone: 1.1 → ---
Henrik pointed out on irc this old bug that sounds like a blocker for one or multiple QA addons to be converted from xul to jetpack!

I took some time to look into widget code, and here is a pontential -simple stupid- way to fix this: prevent customize context menu to be display on all widgets.

Henrik, can you confirm that it is the expected behavior? And that this fix works properly.
Attachment #595371 - Flags: feedback?(hskupin)
Comment on attachment 595371 [details] [diff] [review]
Prevent toolbar context menu to be displayed on widgets

I can confirm that this patch resolved the issue of the toolbar context menu appearing when right clicking on a widget.
Attachment #595371 - Flags: feedback?(hskupin) → feedback+
Comment on attachment 595371 [details] [diff] [review]
Prevent toolbar context menu to be displayed on widgets

Hey dietrich, as you said that you were open to code review on jetpack,
I've found a really cool one for you: a one liner!

Do you think such XUL trick is OK?
"Setting `context` attribute to an empty value so that we won't have a context menu defined on a parent node"
Attachment #595371 - Flags: review?(dietrich)
Comment on attachment 595371 [details] [diff] [review]
Prevent toolbar context menu to be displayed on widgets

Review of attachment 595371 [details] [diff] [review]:
-----------------------------------------------------------------

yes, this is fine. thanks!
Attachment #595371 - Flags: review?(dietrich) → review+
Comment on attachment 595371 [details] [diff] [review]
Prevent toolbar context menu to be displayed on widgets

>+  // Bug 626326: Prevent customize toolbar context menu to appear
>+  node.setAttribute("context", "");

Shouldn't we move down this line to the other node.setAttribute(...) calls? Just for clarity and readability.
Commit pushed to master at https://github.com/mozilla/addon-sdk

https://github.com/mozilla/addon-sdk/commit/9835974d4aa85ca392f14773851a4f8f35a84d8f
Bug 626326: add-on bar should not show context menu when a descendent node was right-click target r=@autonome
We should include this fix in 1.6.
Is there a tag or anything we can add to bugs in order to do such request?
(In reply to Henrik Skupin (:whimboo) from comment #31)
> Shouldn't we move down this line to the other node.setAttribute(...) calls?
> Just for clarity and readability.

Good catch! I've done this in the final patch.
Commit pushed to stabilization at https://github.com/mozilla/addon-sdk

https://github.com/mozilla/addon-sdk/commit/bc99d37ef6c3c1e7f204f65376623307ff71298b
Bug 626326: add-on bar should not show context menu when a descendent node was right-click target r=@autonome
(cherry picked from commit 9835974d4aa85ca392f14773851a4f8f35a84d8f)
:)
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.