Text selection action bar is invoked on long tapping a images in Google Images

RESOLVED FIXED in Firefox 33

Status

()

RESOLVED FIXED
4 years ago
2 years ago

People

(Reporter: TeoVermesan, Assigned: wesj)

Tracking

({reproducible})

32 Branch
Firefox 33
ARM
Android
reproducible
Points:
---

Firefox Tracking Flags

(firefox29 affected, firefox30 affected, firefox31 affected, firefox32 affected, firefox33 verified, fennec+)

Details

Attachments

(2 attachments)

(Reporter)

Description

4 years ago
Created attachment 8434990 [details]
Screenshot_2014-06-05-15-30-47.png

Steps to reproduce:
1. Search an image in google
2. Long tap on it

Expected results:
- The context menu should appear

Actual results:
- The action bar appears
(Reporter)

Updated

4 years ago
status-firefox29: --- → affected
status-firefox30: --- → affected
status-firefox31: --- → affected
status-firefox32: --- → affected
I've seen this too. It makes it difficult to interact with content like in Google Images when the browser keeps trying to text-select.
tracking-fennec: --- → ?
Keywords: reproducible
Summary: Action bar is invoked on long tapping a searched image → Text selection action bar is invoked on long tapping a images in Google Images
Assignee: nobody → wjohnston
tracking-fennec: ? → +
(Assignee)

Comment 2

4 years ago
Hmm.. This is a bit tricky. The issue here is that Google attaches a click handler to the div that's holding the image. We find that div when you tap and use it as our "highlightElement". When you long press, we also use that as our context menu element (i.e. we want the context menu to fire for whatever is highlighted, even if your finger isn't exactly on it):

http://mxr.mozilla.org/mozilla-central/source/mobile/android/chrome/content/browser.js#2294

To fix this we really need to just use anyElementFromPoint. To prevent that from causing the highlighted element to not show a context menu, we can walk up and verify that its a child of highlightElement. Will try...
(Assignee)

Comment 3

4 years ago
Created attachment 8436138 [details] [diff] [review]
Patch
Attachment #8436138 - Flags: review?(margaret.leibovic)

Comment 4

4 years ago
Comment on attachment 8436138 [details] [diff] [review]
Patch

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

anyElementFromPoint and elementFromPoint are confusing, since anyElementFromPoint is the one that actually calls cwu.elementFromPoint! This logic is kinda tricky, but the comments help.

::: mobile/android/chrome/content/browser.js
@@ +2275,5 @@
>      },
>  
> +    _findTarget: function(x, y) {
> +      let isDescendant = function(parent, child) {
> +        var node = child;

Nit: let

@@ +2291,5 @@
> +      let target = BrowserEventHandler._highlightElement;
> +      let touchTarget = ElementTouchHelper.anyElementFromPoint(x, y);
> +
> +      // If we have a highlighted element that has a click handler, we want to ensure our target is inside it
> +      if (touchTarget && target && isDescendant(target, touchTarget)) {

If you wanted, you could omit the check for touchTarget and target here, since isDescendant will return false if either of those elements don't exist.

@@ +2294,5 @@
> +      // If we have a highlighted element that has a click handler, we want to ensure our target is inside it
> +      if (touchTarget && target && isDescendant(target, touchTarget)) {
> +        target = touchTarget;
> +      } else if (!target) {
> +        // Otherwise, lets try to find something clickable

Grammar nit: let's
Attachment #8436138 - Flags: review?(margaret.leibovic) → review+
https://hg.mozilla.org/mozilla-central/rev/c43720109522
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 33
(Reporter)

Comment 7

4 years ago
Verified fixed on:
Device: Alcatel One Touch
OS: Android 4.1
Build: Firefox for Android 33.0a1 (2014-06-12)
(Reporter)

Updated

4 years ago
status-firefox33: --- → fixed
(Reporter)

Updated

4 years ago
status-firefox33: fixed → verified
You need to log in before you can comment on or make changes to this bug.