460966 - mouseover css drop down menu doesn't work with fennec, but works with iphone

Status: RESOLVED FIXED

(Core :: DOM: UI Events & Focus Handling, defect)

Description

[Joel Maher ( :jmaher ) (UTC -8)]

Attachment: simple example to display css drop down menu with a mouseover to trigger it

If I have a css drop down menu on a website (which is very common these days), I expect to see the list drop down when I mouse over the top element.  In the case of fennec, this doesn't happen.  At first I thought this must be a difference between desktop/mobile, but iphone doesn't have this issue.  I have tried this on the desktop version of fennec as well as on the nokia 810.

Comment 1

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

How does it work on the iphone? Does it differentiate "hovers" from "clicks" based on pressure?

Comment 2

[Joel Maher ( :jmaher ) (UTC -8)]

according to nemo, if you tap on a menu (and hold down), it will drop down the  css menu.  Oddly enough all other menus (without a drop down) and links are single tap.  It appears they are intercepting the mouse down/hover event and if something fires, they don't click?  I am downloading the iphone sdk (which has a simulator) to do more research on this.

It sounds like iphone made a user friendly way for simulating mouse interactions.  Not sure how much work it is for us or if it is even possible.

Comment 3

[Joel Maher ( :jmaher ) (UTC -8)]

I downloaded the iphone simulator and see the menu work with a single tap.

Comment 4

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

"Products" in that menu is a link though - when I click it on the desktop it loads another page. Does the iPhone just ignore that click?

Comment 5

[Joel Maher ( :jmaher ) (UTC -8)]

it appears that you need to double click the "products" in order to get to it.  If products didn't have a drop down menu, a single click would work.

also, when you click something in the drop down it works with a single click.

Comment 7

[YUKI "Piro" Hiroshi]

Attachment: patch v1

By this patch, Fennec fires "mosemove" events on any elements if you are not doing pannig.

Comment 8

[YUKI "Piro" Hiroshi]

I saw the bug 492831 (and https://bugzilla.mozilla.org/attachment.cgi?id=382735 ) too, and I tried to update the patch on the bug for this bug. However, I couldn't do it because the patch is designed to fix only for the problem around buttons. BTW, I tested with both patches and looked working fine.

Comment 9

[Ben Combee (:bcombee)]

This is also related to bug 483958.

Comment 10

[YUKI "Piro" Hiroshi]

Attachment: testcase for :hover, :focus, :active

An testcase for :hover, :focus and :active selectors

Comment 11

[YUKI "Piro" Hiroshi]

Results of the testcase on iPhone (Safari):

 * ":hover" is applied if links are clicked. Tapping on <span> doesn't
   set ":hover" state. ":hover" seems available only on links.
 * Both ":focus" and ":active" seems to be ignored on iPhone.

How about we design the behavior around ":hover" selector like as iPhone, importing some codes from DOM Inspector? This is my idea:

 Step 1: Import an code to set ":hover" (and/or ":focus" etc.) state to
         any elements, from DOM Inspector.
 Step 2: Make Fennec to set ":hover" to clicked elements by the API.

This will not fire "mouseover" and "mouseout" events.

Comment 12

[YUKI "Piro" Hiroshi]

Hmm, my idea seems bad. Even if I set ":hover" to the element manually, Fennec set ":hover" to the HTML Canvas (because it is below the pointer) and removes my ":hover" automatically...

Comment 13

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: proposal patch (gecko part)

this is a proposal patch. the behavior is similar to iPhone's. the hover state is set at mouse down event.

Comment 14

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: proposal patch (mobile part)

Comment 15

[Hiroyuki Ikezoe (:hiro)]

I prefer Masayuki's approach. Though there are some more works in Fennec, since css drop down menu is not shown until releasing mouse button due to grabbing mouse down event by panning module.

IMHO, combination of Vivien's efforts in bug 492831 and Masayuki's one will be nice behaviour.

Comment 16

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

Attachment: Dispatch an hover after a certain amount of time

It's nice to see such an activity here!

I want to add this patch to open the discussion on what do we really want here?

This patch dispatch a mouseover after a given amount of time if the user released the mouse/finger without have done any dragging action(this remove the need for looking what have changed and hacking the ESM), but do we want such a behavior and did this conflict with the future (if any) context menu?

(In reply to comment #15)
> I prefer Masayuki's approach. Though there are some more works in Fennec, since
> css drop down menu is not shown until releasing mouse button due to grabbing
> mouse down event by panning module.

Yeah. I like the pref idea too but the stuff that afraid me is the fact that the patch affect chrome UI too, right?

> IMHO, combination of Vivien's efforts in bug 492831 and Masayuki's one will be
> nice behaviour.

What I'm not sure of the way to cancel clicking in the patch for bug 492831, I'm not a huge fan of canceling it only on some kind of nodes, furthermore this lead to a quite complicated events flow :s

Comment 17

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Does Fx on the tablet PCs have this problem (It's impossible to access the contents that are displayed by :hover)?

Comment 18

[Hiroyuki Ikezoe (:hiro)]

From IRC.

mfinkle: vlad: masayuki raises a good question in bug 460966 c17
vlad: the answer is yes

Comment 19

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

Masayuki - The iPhone also sends mouseover and mouseout events. Can we do the same? Post on the iPhone behavior: http://www.quirksmode.org/blog/archives/2008/08/iphone_events.html

I am not yet sure about how closely we want to follow the iPhone model.

Comment 20

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

(In reply to comment #18)
> From IRC.
> 
> mfinkle: vlad: masayuki raises a good question in bug 460966 c17
> vlad: the answer is yes

ok, this should not be a bug of fennec. This should be Core/Event handling.

(In reply to comment #19)
> Masayuki - The iPhone also sends mouseover and mouseout events. Can we do the
> same? Post on the iPhone behavior:
> http://www.quirksmode.org/blog/archives/2008/08/iphone_events.html

I don't read the document yet, but maybe, we can do.

> I am not yet sure about how closely we want to follow the iPhone model.

If we find other better way. We can take it, I think.

Comment 21

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

taking.

Comment 22

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: testcase

iPodtouch generates the events as following order:

* mouseover
* mousemove
* mousedown

# I don't have iPodtouch, tested by Japanese contributor, Irie-san.

Comment 23

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: Patch v1.0

This synthesizes mousemove related events before mousedown event. If the mousemove related events removes the content, this patch aborts the mousedown event.

I think that this can fix this bug. But I don't test this on fennec. Do somebody test this? There are the testbuilds on tryserver:
https://build.mozilla.org/tryserver-builds/[e-mail]-bug460966-6/

I want your feedback, thank you.

Comment 24

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Comment on attachment 397483 [details] [diff] [review]
Patch v1.0

Um... This has some bugs, I'll post new patch.

Comment 25

[Hans Sebastian]

Attachment: screen shot of real example on which options must appear on mouse over

This is a screen shot of a real example to reproduce the bug on how to handle options/elements that must appear on mouse over. The url is http://www.amazon.com, reported on fennec test day.

Comment 26

[Aakash Desai [:aakashd]]

This is a particularly nasty bug on fennec on Amazon.cokm as we're linking to the site from our search engine list (which is very visible on the awesome bar). Is there an update on the patch for this bug?

Comment 27

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Currently, no.

I still don't have better idea. The key points are:

* we need to fire mousemove event before mousedown event
* the patch should be enabled on both Fx and Fennec because the mobile peculiar code shouldn't be there as far as possible.
* needs to pass all tests on tinderbox

Comment 28

[Olli Pettay [:smaug][[e-mail]]]

Because Fennec's event handling is rather bizarre anyway, I'm ok to have
some code which is pref'ed on for Fennec only.

Comment 29

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

o.k. I'll try to refresh my patch.

Comment 30

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: Patch v2.0

I'll create automated tests for this.

Comment 31

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

test build:
https://build.mozilla.org/tryserver-builds/[e-mail]-try-8145c72d92c5/maemo/

Comment 32

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: Patch v3.0

Comment 33

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: Patch v3.1

This patch adds two prefs:

1. ui.touch_device.emulate enables emulation of touch pad device which don't send mousemove related events. This is needed for testing (not automated tests, it means testing at development).

2. ui.touch_device.using enables the new mode. When mousedown event is fired on an element which isn't last element under the cursor, ESM generates a native mousemove event. It will generate mouseover and mouseout events too.

If the element's frame isn't under the cursor after mousemove event is dispatched, ESM stops handling the mousedown event and PresShell doesn't dispatch the DOM events either. By this, the users cannot click unexpected content. Note that even if mousedown event is discarded by the reason, the user can click the moved element again. Then, ESM thinks the element is the last element under the cursor, so, mousedown event will be handled.

Comment 34

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 444631 [details] [diff] [review]
Patch v3.1

> nsEventStateManager::PreHandleEvent(nsPresContext* aPresContext,
>                                     nsEvent *aEvent,
>                                     nsIFrame* aTargetFrame,
>                                     nsEventStatus* aStatus,
>-                                    nsIView* aView)
>+                                    nsIView* aView,
>+                                    PRBool* aStopHandling)
> {
>   NS_ENSURE_ARG_POINTER(aStatus);
>   NS_ENSURE_ARG(aPresContext);
>+  NS_ENSURE_ARG_POINTER(aStopHandling);
No need for this.
If someone calls the method in a wrong way, it is ok to crash.


>+nsresult
>+nsEventStateManager::GenerateMouseMoveForTouchDevice(
>+                       nsPresContext* aPresContext,
>+                       nsMouseEvent* aMouseDownEvent,
>+                       nsEventStatus* aStatus,
>+                       PRBool* aStopHandling)
>+{
>+  NS_PRECONDITION(aStatus && aStopHandling,
>+                  "aStatus and aStopHandling must not be null");
>+  NS_PRECONDITION(!(*aStopHandling),
>+                  "*aStopHandling must be initialized as FALSE");
>+
>+  // If this mouse down event is trused event which was fired by touch device,
trused -> trusted


>+  // we should generate a mouse move event if the target content doesn't have
>+  // :hover state.
Could you explain why not generate move event when :hover is set?

>+  // synthesize a real event because the mousemove event should cause
>+  // a DOM mousemove event.
>+  nsMouseEvent mouseMoveEvent(PR_TRUE, NS_MOUSE_MOVE,
>+                              aMouseDownEvent->widget, nsMouseEvent::eReal);
>+  mouseMoveEvent.time = aMouseDownEvent->time;
>+  mouseMoveEvent.refPoint = aMouseDownEvent->refPoint;
>+  mouseMoveEvent.inputSource = aMouseDownEvent->inputSource;
>+
>+  nsCOMPtr<nsIPresShell> ps(aPresContext->PresShell());
ps doesn't need to be nsCOMPtr

>+  nsCOMPtr<nsIViewObserver> observer = do_QueryInterface(ps);
>+  NS_ENSURE_TRUE(observer, NS_ERROR_FAILURE);
>+
>+  NS_ENSURE_TRUE(!sDispatchingMouseMoveEvent, NS_ERROR_UNEXPECTED);
>+
>+  sDispatchingMouseMoveEvent = PR_TRUE;
>+  observer->DispatchSynthMouseMove(&mouseMoveEvent, PR_TRUE);
>+  sDispatchingMouseMoveEvent = PR_FALSE;
This is a bit hackish, but should work.

This all needs (manual) testing from mobile people.

Comment 35

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: Patch v3.2

Thank you, smaug. I posted the patch to tryserver. The test builds is coming soon.

Comment 36

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Oh, I forgot to post the test build URI.

Here is a new build for latest trunk.
https://build.mozilla.org/tryserver-builds/[e-mail]-try-e4231d86c1a6/

For enabling the new mode, you need to change "ui.touch_device.using" to true.

Somebody can test this build?

Comment 37

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

(In reply to comment #36)
> Oh, I forgot to post the test build URI.
> 
> Here is a new build for latest trunk.
> https://build.mozilla.org/tryserver-builds/[e-mail]-try-e4231d86c1a6/
> 
> For enabling the new mode, you need to change "ui.touch_device.using" to true.
> 
> Somebody can test this build?

I've done some tests with the build provided. 
It doesn't works exactly as I was expected in the specific context of fennec and its canvas based browser, but I can have made a mistake.

Here what I'm doing:
 * go to about:config and change ui.touch_device.using to true
 * go to the testpage set up by jmaher
 * put my finger on the screen and wait
 * release my finger

Actual results:
 * The dropdown is show only when I release my finger and a click is generated

I guess one of the problem here is that we don't send the mousedown event from the canvas surface to the content before the user has release his finger (which generate a click) when the patch expected the mousedown to be dispatched as soon as the mousedown arrived.

Is that what's expected?

Comment 38

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

(In reply to comment #37)
> I guess one of the problem here is that we don't send the mousedown event from
> the canvas surface to the content before the user has release his finger (which
> generate a click) when the patch expected the mousedown to be dispatched as
> soon as the mousedown arrived.
> 
> Is that what's expected?

Yes, it is.

So, there are three approaches:

1. Fennec should fire a mousedown event when an user touches to screen.
2. Fennec should fire a mousemove event when an user touches to screen.
3. Current behavior + my patch.

I'm not sure what is better. If we will take #2, we don't need to land the patch.

I have a question, when you touch to screen and move your finger, what should happen? If that does *not* drag any element, I think that #2 or #3 is better behavior. But if we take #2, we should fire mousemove event when the user moves finger on the screen too. I'm afraid #2 because it might break some features.

Is #3 (i.e., the behavior of the test build) unexpected behavior?

Comment 39

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

(In reply to comment #38)
> (In reply to comment #37)
> > I guess one of the problem here is that we don't send the mousedown event from
> > the canvas surface to the content before the user has release his finger (which
> > generate a click) when the patch expected the mousedown to be dispatched as
> > soon as the mousedown arrived.
> > 
> > Is that what's expected?
> 
> Yes, it is.
> 
> So, there are three approaches:
> 
> 1. Fennec should fire a mousedown event when an user touches to screen.
> 2. Fennec should fire a mousemove event when an user touches to screen.
> 3. Current behavior + my patch.
> 
> I'm not sure what is better. If we will take #2, we don't need to land the
> patch.

I think we don't want to fire mousedown/mousemove when right when the user press the screen because this can cause a reflow and have a bad perf impact on panning.

>
> I have a question, when you touch to screen and move your finger, what should
> happen?

The panning of the canvas area should start without dispatching anything to the content.

> If that does *not* drag any element, I think that #2 or #3 is better
> behavior. But if we take #2, we should fire mousemove event when the user moves
> finger on the screen too. I'm afraid #2 because it might break some features.
> 

> Is #3 (i.e., the behavior of the test build) unexpected behavior?

#3 is actually unexpected behavior because of the canvas surface above the content that redirect mousedown/mouseup events.
I think the problem is that your patch assume the events are dispatched t the content without any intermediate layers, while actually in canvas all the mouse events are swallowed by some code in InputHandler.js which re-create custom mousedown/mouseup events.

I wondering if Fennec will get rid of InputHandler.js with the future Layers. I suspect that we still need to handle the panning ourselves but do we still need to forge raw events? Mark?

Comment 40

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

> I think the problem is that your patch assume the events are dispatched t the
> content without any intermediate layers, while actually in canvas all the mouse
> events are swallowed by some code in InputHandler.js which re-create custom
> mousedown/mouseup events.

Ah, I understand.

Comment 41

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

I don't have better idea for now.

Comment 42

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

it would be nice to get something done here, but this won't block fennec's 2.0 release

Comment 43

[Matt Brubeck (:mbrubeck)]

The test case works in Fennec 4.0b4 and later (thanks to the patch from bug 623765), though you need to press on "Products" and then drag to cancel your click, to avoid activating the "Products" link itself.

Implementing the Safari behavior described in comment 5 (where the first tap triggers the mouseover and the second tap triggers a click) would be nice, but I don't think it's a blocker.

Comment 45

[kolAflash]

Looks like this is still a problem in Firefox 35 for Android. :-(

It also happens on Mozillas own websites, as described here:
https://bugzilla.mozilla.org/show_bug.cgi?id=1068396#c0

Comment 46

[John Nagle]

Mouseover test page: http://unixpapa.com/js/testover.html

This shows mouseover, mouseenter, mouseout, and mouseleave events.  It's a good way to see how touch maps to those events.  On a desktop machine, it behaves as expected, showing mouse enter/leave events as the cursor is moved around.

I'm testing this with Firefox 35.0.1 in the Android emulator running on Linux, where a mouse down simulates a "touch". Clicking/touching within an active area will generate mouseover and mouseenter events. Motion while held down/touching has no effect whatsoever.  This includes leaving the mouse-sensitive area.  Clicking/touching text in a text box selects the text, but does not generate mouseover-type events.  Dragging in an area outside the mouse-sensitive areas causes screen scrolling, without causing mouseout/mouseleave events.

So that's what it does.  Is that what's intended? It's not an unreasonable way to handle touch devices, but some web pages don't play well with it.

I have an add-on which displays icons on web pages.  Mousing over the icon brings up a small tooltip-like widget with some text info. Clicking on the icon, which is a link, takes the user to a page with much more info.  On Firefox on Android, this doesn't work. There's no way to distinguish mouseover from click.  Click has priority, so a new page loads and the tooltip never appears.

This isn't unreasonable, but it should be documented.  Rule: an area can be clickable or mouse-overable on mobile, but not both. This is a constraint on web sites viewable with Firefox.  This seems to explain the problems reported in Bug 1068396. Menus which are both clickable and mouse-overable behave badly.

Comment 47

[John Nagle]

Also tested on an actual Android phone with Firefox 35. Same result.

Comment 48

[oxwivi]

It's Firefox 45 now, still unable to substitute hover on mobile Firefox. The only reason it works in iOS is probably because of WebKit. Chrome forked it into Blink, and it can do the same hover with long-taps.

I'm still looking for it for Firefox on Android. Is there some sort of add-on that can do this for me?

Comment 50

[Snowman25]

Since this Bug is still markes "open" or "new":
With devices that support hover (Like Samsung Galaxy S5), Mouseover-events get triggered by hovering.

Comment 51

[LafinJack]

Anything? Bueller?

Comment 52

[Vincent Lefevre]

This still doesn't work with Firefox 55.0b14 on Samsung Galaxy S7. No such problem with Samsung's Internet browser.

Comment 56

[oxwivi]

Can confirm long tap works as hover on 66.0a1. Thanks to whoever patched support for this and hope to see this on the stable channel soon.

Comment 57

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

I added mousemove event dispatcher at long tap in bug 1514975 because of compatibility with Chrome. However, I'm not sure that this is enough for actual users because doing long tap and canceling context menu with tapping grayed area is not useful steps for most users.

Comment 58

[Kagami Rosylight [:saschanaz] (they/them)]

I think this now "works" but with a need to dismiss context menu as comment #57 says. Let's track the menu issue in Bug 1650858.