Closed Bug 891175 Opened 9 years ago Closed 9 years ago

Orange caret indicator is off by a couple lines

Categories

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

Other
Android
defect
Not set
normal

Tracking

(firefox24 unaffected, firefox25+ verified, fennec25+)

VERIFIED FIXED
Firefox 25
Tracking Status
firefox24 --- unaffected
firefox25 + verified
fennec 25+ ---

People

(Reporter: alex_mayorga, Assigned: kats)

References

Details

(Keywords: regression)

Attachments

(5 files)

The caret indicator is off by a couple lines, please se attached.
Which build/device/OS? Reproducible on all forms, or only that Persona login? Steps to reproduce?
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → INCOMPLETE
This is happening everywhere, even as I type this bugzilla comment it also happens on m.facebook.com

The screenshots are obtained with Nightly updated as of today running on Free Xperia Project CyanogenMod 10 on a Sony Ericsson Xperia pro.

Please let me know if there's anything needed from the phone.
Status: RESOLVED → REOPENED
Resolution: INCOMPLETE → ---
Is this a regression? Does it happen on Aurora/Beta/Release?
Seems it is a regression as it doesn't reproduce on Release/Beta/Aurora on this phone.
Let's see if we can get a regression window.
Component: General → Text Selection
I see this too in Nightly. It makes the cursor caret pretty much unusable.
Status: REOPENED → NEW
tracking-fennec: --- → ?
The regression window is:

1.mozilla-central
good build: 04.07.2013 
bad build: 05.07.2013 
-pushlog:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=dcbbfcdf7bb4&tochange=17fe59f6c54a

2.inbound
good build:1372942018
bad build: 1372943362
-pushlog: 
http://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=0dbc3d79971d&tochange=efa23d6bd3fa
Blocks: 803207
Looks like DOMWindowUtils.sendQueryContentEvent returns a thing in LayoutDevice pixels, not CSS pixels. We'll have to divide cursor.* by window.devicePixelRatio before using it at [1]. The code at [2] may also be affected, although I'm not entirely sure how that will manifest. We should also update the documentation for sendQueryContentEvent to indicate that it returns things in LayoutDevicePixel space rather than CSSPixel space.

[1] http://mxr.mozilla.org/mozilla-central/source/mobile/android/chrome/content/SelectionHandler.js#500
[2] http://mxr.mozilla.org/mozilla-central/source/mobile/android/chrome/content/SelectionHandler.js#334
CC'ing mbrubeck in case this affects metro as well. Looks like metro also uses these functions.
Attached patch PatchSplinter Review
Haven't tried this locally yet (in the middle of other stuff) but I pushed it to try and will test out the build when that's done. If that works I'll flag for review.

https://tbpl.mozilla.org/?tree=Try&rev=95b3ab7b2b7c
tracking-fennec: ? → 25+
Assignee: nobody → bugmail.mozilla
Comment on attachment 774000 [details] [diff] [review]
Patch

Checked the try build, this seems to work as expected.
Attachment #774000 - Flags: review?(mbrubeck)
Comment on attachment 774000 [details] [diff] [review]
Patch

Passing review to jimm -- he knows the Metro version of this best, though neither of us really know this Android code.  (Maybe margaret would like to review it too?)
Attachment #774000 - Flags: review?(mbrubeck) → review?(jmathies)
Comment on attachment 774000 [details] [diff] [review]
Patch

I understand what you're doing here and it looks correct, but I have no way to test it.
Attachment #774000 - Flags: review?(jmathies) → review+
Attachment #774000 - Flags: review?(margaret.leibovic)
Comment on attachment 774000 [details] [diff] [review]
Patch

This looks good to me. Maybe add some comments where we call sendQueryContentEvent explaining why we need to do this conversion (this code is always kinda confusing, so I like to err on the side of more comments).
Attachment #774000 - Flags: review?(margaret.leibovic) → review+
https://hg.mozilla.org/mozilla-central/rev/0de26cfc54bd
Status: NEW → RESOLVED
Closed: 9 years ago9 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 25
The caret is now on the right place in Nightly.

Thank you!
Status: RESOLVED → VERIFIED
Verified fixed on:
Build: Firefox for Android 25.0a1(2013-07-14)
Device: Samsung Galaxy Tab
OS: Android 4.0.4
Duplicate of this bug: 904653
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.