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

RESOLVED FIXED

Status

Add-on SDK
General
P2
normal
RESOLVED FIXED
6 years ago
3 years ago

People

(Reporter: dietrich, Assigned: dietrich)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [addon bar])

Attachments

(3 attachments)

(Assignee)

Description

6 years ago
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.
(Assignee)

Comment 1

6 years ago
spun off from https://bugzilla.mozilla.org/show_bug.cgi?id=624607#c2.
(Assignee)

Updated

6 years ago
Whiteboard: [addon bar]
(Assignee)

Comment 2

6 years ago
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

Comment 3

6 years ago
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.
(Assignee)

Comment 5

6 years ago
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.

Comment 7

6 years ago
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.
(Assignee)

Comment 8

6 years ago
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
(Assignee)

Updated

6 years ago
Whiteboard: [addon bar][softblocker] → [addon bar]

Comment 9

6 years ago
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
(Assignee)

Comment 11

6 years ago
Created attachment 510125 [details]
Pointer to pull request
(Assignee)

Comment 12

6 years ago
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.
Component: General → General
Product: Add-on SDK → Firefox
QA Contact: general → general
Target Milestone: 1.0b3 → ---
--> Moving back.
blocking2.0: final+ → ---
Component: General → General
Product: Firefox → Add-on SDK
QA Contact: general → general
(Assignee)

Updated

6 years ago
Attachment #510125 - Flags: review?(adw)
Assignee: nobody → dietrich
Target Milestone: --- → 1.0b3

Comment 15

6 years ago
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+
(Assignee)

Comment 16

6 years ago
Yep, will fix comment 7 in this bug too.
(Assignee)

Comment 17

6 years ago
(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.
(Assignee)

Comment 18

6 years ago
Created attachment 515930 [details]
Pointer to pull request
(Assignee)

Comment 19

6 years ago
latest attachment fixes adw's comments, but not the add-on bar context menu bit.
(Assignee)

Updated

6 years ago
Attachment #515930 - Flags: review?(adw)
(Assignee)

Comment 20

6 years ago
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 21

6 years ago
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)
(Assignee)

Comment 22

6 years ago
(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.
(Assignee)

Comment 23

6 years ago
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.
Blocks: 634712
Priority: -- → P3
Target Milestone: 1.0b3 → 1.0
(automatic reprioritization of 1.0 bugs)
Priority: P3 → P2
Target Milestone: 1.0 → 1.1

Comment 25

6 years ago
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 → ---
Created attachment 595371 [details] [diff] [review]
Prevent toolbar context menu to be displayed on widgets

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

Comment 30

5 years ago
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.

Comment 32

5 years ago
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.

Comment 35

5 years ago
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
Last Resolved: 5 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.