Closed Bug 891175 Opened 9 years ago Closed 9 years ago
Orange caret indicator is off by a couple lines
73.73 KB, image/png
37.30 KB, image/png
61.62 KB, image/png
52.91 KB, image/png
3.86 KB, patch
|Details | Diff | Splinter Review|
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.
I see this too in Nightly. It makes the cursor caret pretty much unusable.
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
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 . The code at  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.  http://mxr.mozilla.org/mozilla-central/source/mobile/android/chrome/content/SelectionHandler.js#500  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.
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
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+
9 years ago
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+
Added the comments and landed: https://hg.mozilla.org/integration/mozilla-inbound/rev/0de26cfc54bd
Status: NEW → RESOLVED
Closed: 9 years ago → 9 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
You need to log in before you can comment on or make changes to this bug.