This behaviour is different from Stable or Chrome. (Stable ignores it, Chrome implements it correctly) May be due to this apparently. https://mail.mozilla.org/pipermail/mobile-firefox-dev/2015-December/001645.html When using the stylus, you can see a little hover point on the screen as you move it around. This helps you see what you are about to target on the screen - it is also great for anything that requires hover. It kind of acts as if you're using a little mouse, you can precisely target links, easily navigate menus in desktop apps in VNC, and use any menu on a site that uses :hover events w/o difficulty. Stable does not seem to pay any attention to this, so if you move over a hover menu on a site you still need to touch down to get it to do something. Chrome however fires off the appropriate hover. Nightly by contrast acts as if I'm touching down the whole time. This causes things like triggering selection, opening links, and scrolling wildly.

I suspect this is a combination of APZ being enabled and bug 1224604, which started treating hover events as touch.

Interesting, when I patched the accessibility issue I hadn't considered that hover was possible on a touch device. I'm sure there are other parts of the input handling where this is the case too - https://dxr.mozilla.org/mozilla-central/source/widget/android/AndroidJavaWrappers.cpp?from=widget%2Fandroid%2FAndroidJavaWrappers.cpp#606 it looks like if the tool type isn't a mouse then a touch event is generated.

(In reply to sgiles from comment #2) > I'm sure there are other parts of the > input handling where this is the case too - > https://dxr.mozilla.org/mozilla-central/source/widget/android/ > AndroidJavaWrappers.cpp?from=widget%2Fandroid%2FAndroidJavaWrappers.cpp#606 > it looks like if the tool type isn't a mouse then a touch event is generated. Right, so that code is used when sending the event to gecko. Based on my reading of the code what I think is happening is: - With JPZC, the hover events are ignored in the JPZC code, but get sent to gecko as touch events - With C++ APZ, the hover events are converted to touch events and then sent through both APZ and Gecko The accessibility code probably relies on gecko getting the hover events as touch events. However, this is technically incorrect (because web content will see them touch events as well) and we should probably change how this works. However.. now I'm wondering again why the crash in bug 1224604 was even happening. According to comment 1 on that bug the crash was happening because the hover events were being sent to gecko and not getting handled in the multitouch function - do you remember what the backtrace looked like there? The onHoverEvent function in LayerView should call sendEventToGecko which should be sending the event as a MOTION_EVENT to the native code, and that should have been going through the existing AndroidGeckoEvent::MakeTouchEvent conversion code.

NI'ing some a11y people in case that code should change.

WRT comment #2 .. as well as stylus, Samsung also has bug #1013899 for hovering. And, in support for stylus in general, which Firefox never did that well, as well as hover there's bug #1213756 (smaller area, which would make touching links precisely much better) and bug #808591 (gestures using stylus which would be darn cool) Also, I found another stylus hover bug.... bug #1125105 - this one is odd 'cause it is happening in stable. I'm going to guess that that site is simply not handling the hover events properly except... well, firefox stable sure didn't seem to pass hover events on before. So surely some other non-hover event is being passed on? I bring this up 'cause bug #1125105, comment #5 seems to be a misunderstanding of how the hover works w/ the s-pen then...

kats, I just backed out the patch on my local tree, and it doesn't appear to crash reliably anymore.. it definitely used to. I added some log outputs in gfx/LayerView.java and AndroidJavaWrappers.cpp (in MakeTouchEvent and MakeMultiTouchEvent): ``` I/GeckoLayerView( 5967): DEBUG onHoverEvent LayerView I/GeckoLayerView( 5967): DEBUG onGenericMotionEvent LayerView I/GeckoLayerView( 5967): DEBUG onHoverEvent LayerView I/GeckoLayerView( 5967): DEBUG onGenericMotionEvent LayerView I/GeckoLayerView( 5967): DEBUG onHoverEvent LayerView I/GeckoLayerView( 5967): DEBUG onGenericMotionEvent LayerView ``` This was generated with a single 'tap' with TalkBack on. Has something changed following that patch?

I don't have a backtrace from the other bug anymore, but it ended up on this line: https://dxr.mozilla.org/mozilla-central/source/mobile/android/base/java/org/mozilla/gecko/gfx/NativePanZoomController.java#81

Oh yeah, that's right. So - I think it makes sense to back out the patch, and let those hover events use the old path go get delivered to gecko directly without going through APZ. We can remove the exception at the line you referenced, since it is valid for it to get called in this context, but we don't need to do anything about it.

BTW, this bug also seems to impact the B2G Firefox Developer Preview launcher for Android. It's interesting 'cause when this launcher was first announced, it crashed every single time I tried to use the stylus, but since there was no crash reporting, I couldn't easily look into why. I wonder if it was crashing on that forced exception throwing that sgiles linked above. Anyway, now it doesn't crash anymore, but is still pretty unusable w/ the stylus for same reason as the browser.

I can do what I said in comment 8.

I have a patch, but my build is breaking because of an unrelated reason. Will upload.

Ok, I managed to get my build working - it doesn't crash with the accessibility stuff \o/

Comment on attachment 8697300 [details] [diff] [review] hover-event-handling.patch Review of attachment 8697300 [details] [diff] [review]: ----------------------------------------------------------------- Cool, thanks! ::: mobile/android/base/java/org/mozilla/gecko/gfx/NativePanZoomController.java @@ +75,5 @@ > } > > @Override > public void notifyDefaultActionPrevented(boolean prevented) { > + // no-op Update this to say // This could get called if accessibility is enabled and the events are sent to Gecko directly without going through APZ. In this case we just want to ignore this callback.

Adding r= and updated to include comment. Check in please!

Comment on attachment 8697321 [details] [diff] [review] hover-events-accessibility.patch Review of attachment 8697321 [details] [diff] [review]: ----------------------------------------------------------------- ::: widget/android/AndroidJavaWrappers.cpp @@ -684,5 @@ > int endIndex = Count(); > > switch (Action()) { > - case AndroidMotionEvent::ACTION_HOVER_ENTER: { > - if (ToolTypes()[0] == AndroidMotionEvent::TOOL_TYPE_MOUSE) { Maybe this could be: if (ToolTypes()[0] != AndroidMotionEvent::TOOL_TYPE_FINGER) If this is removed completely I think it will break accessibility.

Note that there's two different functions that look very similar and that have that code. The one that creates a WidgetTouchEvent will remain as-is, this one returns a MultiTouchInput and is used for the APZ.

Just tried it in Nightly on my Note 4 - and indeed, no more wild scrolling. However, I was disappointed, 'cause I'd gotten the impression the hover events would be passed on to the page. But the behaviour seems to be back to stable Firefox, not Chrome. Hovering over a dropdown menu does nothing. So, yeah, I guess "fixed" but still not full stylus handling.

Yeah, sorry. Proper stylus handling will require more work.