575586 - [LongTap]Fennec does not pass long tap event to plug-ins

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Ilkka Otsala]

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.0.19) Gecko/2010040119 Ubuntu/8.04 (hardy) Firefox/3.0.19
Build Identifier: Mozilla/5.0 (X11; U; Linux armv7l; en-US; rv:1.9.3a5pre) Gecko/20100622 Firefox/ Fennec/2.0a1pre

If a long tap gesture is done on top of e.g. flash content, a right click event should be passed to plug-in for context sensitive menu purposes.

Reproducible: Always

Steps to Reproduce:
1. Load page that contains flash e.g. youtube.com 
2. make long tap gesture on top of flash content

Actual Results:  
Nothing happens.

Expected Results:  
Flash context sensitive menu should be popped up.

Comment 1

[Jan D]

I suppose it would be useful for context sensitive add-ons too: They could alter the context menu depending on context.

Comment 2

[Jan Arne Petersen]

Attachment: Forward long tap event to plugins

Comment 3

[Mark Finkle (:mfinkle) (use needinfo?)]

Waiting until we have plugins working in Fennec again before reviewing this patch.

Comment 4

[Brad Lassey [:blassey] (use needinfo?)]

not blocking fennec, but we'll take a patch if and when plugins work and it can be tested.

Comment 5

[Antti Järvelin]

Attachment: Forward long tap event to plugins V2

This is an update to the previous patch. It should work with the Fennec's updated CSM handling code.

However, I believe that csm for embedded elements, like flash, will still be broken when the embedded element is located at a position which needs panning.  I think that fix for the bug 607881 (or similar) could resolve the issue.

Comment 6

[Oleg Romashin (:romaxa)]

Attachment: Small and simple patch for content process CSM flash handling

Plugins are testable...
Install youtube enabler extension or just enable it with prefs

Comment 7

[Oleg Romashin (:romaxa)]

Don't know is it blocking or depends from bug 625673

Comment 8

[Oleg Romashin (:romaxa)]

Attachment: maemo only now, and use sendMouseEvent

Comment 9

[Oleg Romashin (:romaxa)]

Attachment: Magic Start for filter ifdef

Comment 10

[Matt Brubeck (:mbrubeck)]

Comment on attachment 503852 [details] [diff] [review]
maemo only now, and use sendMouseEvent

>+++ b/chrome/content/content.js

>+        let popupNode = element;

Why create a new variable?  Looks like you should just use "element".

>+        let embeddedNode =
>+          popupNode.nodeType == Ci.nsIDOMNode.ELEMENT_NODE &&
>+          popupNode instanceof Ci.nsIDOMHTMLEmbedElement;
>+        if (embeddedNode) {

Do you need both checks here, or could you just do "if (element instanceOf Ci.nsIDOMHTMLEmbedElement)"?

>+    if (!aButton) {
>+      aButton = 0;
>+    }

Nit: Please remove the {}, and move this block to the very top of the function.

r=mbrubeck with those changes.

Comment 11

[Mark Finkle (:mfinkle) (use needinfo?)]

Comment on attachment 503852 [details] [diff] [review]
maemo only now, and use sendMouseEvent

>diff --git a/chrome/content/content.js b/chrome/content/content.js

>+    if (!aButton) {
>+      aButton = 0;
>+    }

Change to:
aButton = aButton || 0;

Comment 12

[Oleg Romashin (:romaxa)]

Attachment: Updated patch

Comment 13

[Oleg Romashin (:romaxa)]

http://hg.mozilla.org/mobile-browser/rev/eee69f727a24