766556 - Text-selection handles separate from selection on device rotation

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: Text Selection, defect)

Description

[Aaron Train [:aaronmt]]

Currently when one selects a word, sentence or paragraph of text and just so happens to rotate ones device, the selection will remain (off-screen), but the selection handles will float elsewhere are are not at their new position 'tethered' to the selection.

Comment 1

[:Margaret Leibovic]

This should be pretty simple to fix if we can listen for device rotation in JS. We'd just need to check if there's an active selection, then call updateCacheForSelection(), updateCacheOffset(), and positionHandles() (based off my changes in bug 765057).

Comment 2

[:Margaret Leibovic]

Attachment: hacky patch

I tried finding a web API to use for this, but the only one that was working for me is the device orientation API, and that doesn't give us useful information. What we really want to listen for is when the content needs to re-draw to fit different dimensions, since there are other similar cases that will cause problems, like when the tabs sidebar opens on tablets.

This patch might be a good temporary fix, but it's kinda gross. I found that I needed to wait a little bit before re-positioning the handles to let the page sort itself out, but I didn't test this on more complicated websites.

Comment 3

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

Comment on attachment 635533 [details] [diff] [review]
hacky patch

Have you tried using the "resize" event in browser.js ?

window.addEventListener("resize", ...)

You will get that when the keyboard appears/disappears too, but that should be fine.

Comment 4

[Kartikaya Gupta (email:[e-mail])]

If you need to listen for resize events, consider listening for the Window:Resize notification instead of window.resize. See bug 764467 for why. It may not apply here, but it may avoid problems later.

Comment 5

[:Margaret Leibovic]

Attachment: patch using Window:Resize

This patch is better in that it listens for what we actually care about. However, I found I still needed the setTimeout, and I'm worried that it could sometimes still fail on pages that take a longer time to draw or on slower devices (I'm testing on a Nexus S). We could make the delay longer, but then that makes us look slow on faster devices.

Is there any way to listen for the page finishing drawing?

Comment 6

[:Margaret Leibovic]

Comment on attachment 635834 [details] [diff] [review]
patch using Window:Resize

I found this patch has problems if endSelection() is called before the timeout callback executes. We should check in there as well to make sure there's still an active selection if we're going to go with this approach.

Comment 7

[:Margaret Leibovic]

Attachment: patch v2

(Built on top of my patch queue)

This feels good enough for a first attempt at fixing this issue. We can always refine it later.

Comment 8

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

The setTimeout(..., 200) has me wondering how it will work on slower devices

Comment 9

[:Margaret Leibovic]

Attachment: alternate quick-fix patch

The stock browser just cancels the selection when you rotate the device, and that seems like a good enough quick-fix for us. I can file a follow-up bug to support re-positioning the handles when the device rotates.

Comment 10

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

Comment on attachment 637653 [details] [diff] [review]
alternate quick-fix patch

I think this is fine for now. File a followup so we remember to try a better approach later.

Comment 11

[:Margaret Leibovic]

https://hg.mozilla.org/integration/mozilla-inbound/rev/2e079d4b54b9

Comment 12

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/2e079d4b54b9

Comment 13

[:Margaret Leibovic]

Uplifted to aurora as part of a roll-up patch:
https://hg.mozilla.org/releases/mozilla-aurora/rev/2fb0a358eaf6

Comment 14

[Aaron Train [:aaronmt]]

Verified that the handles re-position themselves back after rotation

Comment 15

[Adrian Tamas (:AdrianT)]

The fix was never uplifted to Beta (issue is reproducible on both 15.0b2 and 15.0b3). New follow-up issue logged - please see bug 779811

Comment 17

[Aaron Train [:aaronmt]]

Please nominate for Beta.

Comment 18

[:Margaret Leibovic]

This didn't need to land on beta because when it landed on Aurora, Aurora was still Firefox 15. Looking at the code, this fix is in beta:
https://hg.mozilla.org/releases/mozilla-beta/file/tip/mobile/android/chrome/content/browser.js#l1501

Adrian, what exactly are you seeing? The selection should end when you rotate the device. If not, you are seeing a new bug, possibly introduced by bad uplift interactions.