Text selection "cursor" handle persists across tab switches

VERIFIED FIXED in Firefox 20

Status

()

Firefox for Android
General
VERIFIED FIXED
5 years ago
11 months ago

People

(Reporter: kats, Assigned: Ravisankar Sivasubramaniam)

Tracking

unspecified
Firefox 20
All
Android
Points:
---

Firefox Tracking Flags

(firefox20 verified)

Details

(Whiteboard: [mentor=margaret][lang=js])

Attachments

(1 attachment, 2 obsolete attachments)

STR:
1. Open a tab with some page (e.g. mozilla.org)
2. In a new tab, go to a page with an input field (e.g. bugzilla.mozilla.org)
3. Click on the input field (and maybe scroll the page a bit) so that the orange cursor image is visible
4. Switch tabs

Expected results:
The orange cursor image disappears

Actual results:
The orange cursor image is overlaid on the other tab (mozilla.org)

Updated

5 years ago
Blocks: 652168

Comment 1

5 years ago
Looks like we only get rid of selection handles on a "Tab:Selected" event, not get rid of the cursor as well:
http://mxr.mozilla.org/mozilla-central/source/mobile/android/chrome/content/browser.js#1652

We should add a this.hideThumb() call if _activeType is TYPE_CURSOR in this case.
Whiteboard: [mentor=margaret][lang=js]
(Assignee)

Comment 2

5 years ago
Hi Margaret,

Would you please assign this bug to me and send me instruction to fix it?

Thanks,
Ravi

Updated

5 years ago
Assignee: nobody → rasivasu
Flags: needinfo?(margaret.leibovic)

Comment 3

5 years ago
(In reply to Ravisankar Sivasubramanaim from comment #2)

> Would you please assign this bug to me and send me instruction to fix it?

Sure! Do you have a build environment set up? If not, you should follow the instructions here:
https://wiki.mozilla.org/Mobile/Fennec/Android#Building_Fennec

After that, you should see if you can reproduce the issue on your own build, then you'll just need to add a call to hide the thumb if it's active when we observe a "Tab:Selected" event in SelectionHanlder's observe method.

Find me on IRC if you have issues! Sorry I was a bit slow to respond last time :(
Flags: needinfo?(margaret.leibovic)
(Assignee)

Comment 4

5 years ago
Created attachment 682872 [details] [diff] [review]
Added code to hide thumb for cursor.
Attachment #682872 - Flags: review?(margaret.leibovic)
(Assignee)

Comment 5

5 years ago
Created attachment 682886 [details] [diff] [review]
Added code to hide thumb for cursor.
Attachment #682886 - Flags: review?(margaret.leibovic)
(Assignee)

Comment 6

5 years ago
Hi Margaret,

Would you please review this patch?

Thanks,
Ravi
Status: NEW → ASSIGNED

Comment 7

5 years ago
Comment on attachment 682872 [details] [diff] [review]
Added code to hide thumb for cursor.

This looks like an accidental attachment. Marking as obsolete. I'll review the patch that was attached :)
Attachment #682872 - Attachment is obsolete: true
Attachment #682872 - Flags: review?(margaret.leibovic)

Comment 8

5 years ago
Comment on attachment 682886 [details] [diff] [review]
Added code to hide thumb for cursor.

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

Awesome! Thanks for the patch! I just have one small comment. If you upload a new patch with that change I can land it for you. Also, please read this page about how to properly format your patch so that it's ready to be checked in:
https://developer.mozilla.org/en-US/docs/Creating_a_patch_that_can_be_checked_in

::: mobile/android/chrome/content/browser.js
@@ +1660,5 @@
>            // Knowing when the page is done drawing is hard, so let's just cancel
>            // the selection when the window changes. We should fix this later.
>            this.endSelection();
> +        } else if (this._activeType == this.TYPE_CURSOR) {
> +          //  Hide the cursor if a new window is selected

Nit: Update this comment to "Hide the cursor if the window changes." since this code also runs for the "Window:Resize" event.
Attachment #682886 - Flags: review?(margaret.leibovic) → review+
(Assignee)

Comment 9

5 years ago
Created attachment 684432 [details] [diff] [review]
Added code to hide the cursor if the windows change
Attachment #684432 - Flags: review?(margaret.leibovic)

Updated

5 years ago
Attachment #684432 - Flags: review?(margaret.leibovic) → review+

Updated

5 years ago
Attachment #682886 - Attachment is obsolete: true

Comment 10

5 years ago
Thanks for the patch! I landed it on mozilla-inbound, so it will be merged into mozilla-central within a day, and it will be part of Firefox 20!

https://hg.mozilla.org/integration/mozilla-inbound/rev/45c61a3e9678
https://hg.mozilla.org/mozilla-central/rev/45c61a3e9678
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 20
The cursor handle doesn't persist any more on the latest Nightly build. Closing bug as verified fixed on:

Firefox 20.0a1 (2012-11-28)
Device: Galaxy Nexus
OS: Android 4.1.1
Status: RESOLVED → VERIFIED
status-firefox20: --- → verified
You need to log in before you can comment on or make changes to this bug.