Closed Bug 766556 Opened 8 years ago Closed 8 years ago

Text-selection handles separate from selection on device rotation

Categories

(Firefox for Android :: Text Selection, defect)

ARM
Android
defect
Not set

Tracking

()

VERIFIED FIXED
Firefox 16
Tracking Status
firefox15 --- affected
firefox16 --- verified
firefox17 --- verified

People

(Reporter: aaronmt, Assigned: Margaret)

References

Details

Attachments

(2 files, 2 obsolete files)

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.
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).
Attached patch hacky patch (obsolete) — Splinter Review
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.
Assignee: nobody → margaret.leibovic
Attachment #635533 - Flags: feedback?(mark.finkle)
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.
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.
Attached patch patch using Window:Resize (obsolete) — Splinter Review
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?
Attachment #635533 - Attachment is obsolete: true
Attachment #635533 - Flags: feedback?(mark.finkle)
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.
Attached patch patch v2Splinter Review
(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.
Attachment #635834 - Attachment is obsolete: true
Attachment #635986 - Flags: review?(mark.finkle)
The setTimeout(..., 200) has me wondering how it will work on slower devices
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.
Attachment #637653 - Flags: review?(mark.finkle)
Attachment #635986 - Flags: review?(mark.finkle)
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.
Attachment #637653 - Flags: review?(mark.finkle) → review+
Depends on: 769449
https://hg.mozilla.org/mozilla-central/rev/2e079d4b54b9
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Uplifted to aurora as part of a roll-up patch:
https://hg.mozilla.org/releases/mozilla-aurora/rev/2fb0a358eaf6
Component: General → Text Selection
Verified that the handles re-position themselves back after rotation
Status: RESOLVED → VERIFIED
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
Depends on: 779811
Duplicate of this bug: 779811
Please nominate for Beta.
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.
tracking-fennec: ? → ---
You need to log in before you can comment on or make changes to this bug.