810821 - Tapping on url bar with Samsung Galaxy Note's II stylus doesn't have url text selected

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: Keyboards and IME, defect)

Description

[Martijn Wargers (dead)]

Steps to reproduce:
- Open Fennec
- Tap on url bar with the Galaxy Note II pen

Expected result:
- Text in url bar is selected (e.g. about:home text)

Actual result:
- Text in url bar is not selected, caret is at the end

A friend of mine found this out, I didn't actually test this.
He tested this with the release version of Fennec.

Comment 1

[Martijn Wargers (dead)]

This is also an issue in the latest Nightly build.

Comment 2

[rclement42]

Here's what I noticed, I can select the text in the search box but the second I move the S-Pen back to the handwriting pad (just when it hovers above it - blue circle), then the text gets deselected and caret goes to the end!

It's driving me nuts, because it means I have to delete the word manually instead of overwriting the selected text.

Comment 3

[Martijn Wargers (dead)]

This is rather a deal breaker when trying to use Fennec on this device.

Comment 4

[Kevin Brosnan [Ex-Mozilla]]

Doubt release management is looking to track for a specific release. Setting tracking Fennec for comment 3.

Comment 5

[Alex Keybl [:akeybl]]

The first opportunity after device launch that we have the opportunity to fix is FF18. But you're right kbrosnan, it's very unlikely that we'd block ship on this bug.

Comment 6

[Aaron Train [:aaronmt]]

I've got a Note 2 in Toronto.

Comment 7

[rclement42]

It works well with Skyfire and Chrome (Dolphin also has occasional issues, but the X in the address bar helps out with that). 

Also, turning off Air View does not solve the problem.  The issue is really only when using the S-pen since finger is fine.

Comment 8

[Aaron Train [:aaronmt]]

I can't reproduce this; what am I missing here?

On the Note II (Android 4.2), I use the pen and tap the URL-bar, and about:home becomes highlighted. The same thing happens on any device with a regular finger-tap.

Comment 9

[rclement42]

I'm on Android 4.1.1.  After you tap and select the text in the URL, if you move the pen back to the keyboard to type something new in, the second the pen hovers over the keyboard, the highlighted text in the URL gets de-selected.  If I use my finger to type instead, it's fine (but usually if I'm holding the pen, I tend to tap the input with it as well, especially useful in the winter).

Comment 10

[(inactive) Jim Chen [:jchen] [:darchons]]

(In reply to rclement42 from comment #9)
> I'm on Android 4.1.1.  After you tap and select the text in the URL, if you
> move the pen back to the keyboard to type something new in, the second the
> pen hovers over the keyboard, the highlighted text in the URL gets
> de-selected.  If I use my finger to type instead, it's fine (but usually if
> I'm holding the pen, I tend to tap the input with it as well, especially
> useful in the winter).

Can you install this nightly, http://people.mozilla.org/~nchen/bug810821-fennec-20.0a1.apk, reproduce the bug, and upload the logcat output (see https://wiki.mozilla.org/Android#Using_logcat)?

Comment 11

[rclement42]

(In reply to Jim Chen [:jchen :nchen] from comment #10)

I got the alogcat app, opened the nightly, started the log, then reproduced the bug but I'm not sure everything went right (https://docs.google.com/open?id=0B1owMaZGNjDdbDVseFlXSDllcTA @ 22:57).

First time I try this but I'm definitely willing to help, so let me know.

Comment 12

[(inactive) Jim Chen [:jchen] [:darchons]]

(In reply to rclement42 from comment #11)
> (In reply to Jim Chen [:jchen :nchen] from comment #10)
> 
> I got the alogcat app, opened the nightly, started the log, then reproduced
> the bug but I'm not sure everything went right
> (https://docs.google.com/open?id=0B1owMaZGNjDdbDVseFlXSDllcTA @ 22:57).
> 
> First time I try this but I'm definitely willing to help, so let me know.

Thanks for your help! Looks like the aLogcat app does not support Android 4.1 without rooting the phone. The easier way is probably to just connect your phone to your computer and then use the adb tool

If you are on Windows,
1) Turn on Developer options in Settings, and enable USB debugging
2) Connect your phone to your computer through USB (you may have to install drivers for your phone)
3) Follow the direction here to download adb, http://www.keyables.com/2012/06/how-to-install-and-use-adb-tool.html#mini-adb
4) Once you reach the command prompt, enter "adb logcat -c"
5) Now launch Nightly on your phone and reproduce the bug
6) Go back to the command prompt, and this time enter "adb logcat -d > log.txt"
7) Attach the log.txt file that should be in the same directory as adb

Let me know if you need clarification on any of the steps, or if you're using OS X or Linux. Thanks again for your help!

Comment 13

[rclement42]

(In reply to Jim Chen [:jchen :nchen] from comment #12)

Got to the cmd window (Kies is installed on my PC and my phone is detected (also shows up in device manager)), but for some reason adb devices returns; "List of devices attached" empty.  Tried killing and re-starting the server, rebooting, unplugging/replugging phone to no avail (USB debugging is turned on).  I'll try on a different computer when I can (this is WinXP?).

Comment 14

[Kevin Brosnan [Ex-Mozilla]]

You need the ADB USB driver for Samsung devices. http://downloadcenter.samsung.com/content/SW/201211/20121110030255344/Samsung_USB_Driver_for_Mobile_Phones_v1.5.14.0.exe

Comment 15

[rclement42]

But Kies installs the ADB USB driver; it's definitely there - see it in the Device Manager and I can view and explore my device.

In fact, downloading from your link and trying to run gives me the "already installed" message.

Comment 16

[Kevin Brosnan [Ex-Mozilla]]

Have you enabled USB debugging on the device?

Comment 17

[rclement42]

(In reply to Kevin Brosnan [:kbrosnan] from comment #16)
> Have you enabled USB debugging on the device?

Please refer to comment #13.

I've read quite a few threads concerning this issue, so I'll try a few things tomorrow and post back

Comment 18

[rclement42]

(In reply to Jim Chen [:jchen :nchen] from comment #12)

Wow, that was a nightmare.  I had the hardest time getting the phone detected.  Windows could see it fine but the list of devices was always returned empty.  I tried three different drivers, rebooted/re-started, different USB modes, toggling, different O/S, different computers and every trick I could find in any forums - I was *this close* to freaking out.

Anyways, as a last resort, I installed the full developper tools and lo and behold; it worked.  Anyways - the log is at http://db.tt/FxcefVa3 (I tried to get time stamps, but wasn't able to with the -v time option).

Comment 19

[(inactive) Jim Chen [:jchen] [:darchons]]

(In reply to rclement42 from comment #18)
> (In reply to Jim Chen [:jchen :nchen] from comment #12)
> 
> Wow, that was a nightmare.  I had the hardest time getting the phone
> detected.  Windows could see it fine but the list of devices was always
> returned empty.  I tried three different drivers, rebooted/re-started,
> different USB modes, toggling, different O/S, different computers and every
> trick I could find in any forums - I was *this close* to freaking out.
> 
> Anyways, as a last resort, I installed the full developper tools and lo and
> behold; it worked.  Anyways - the log is at http://db.tt/FxcefVa3 (I tried
> to get time stamps, but wasn't able to with the -v time option).

Thank you very much for your time spent! Can you try using this Nightly? http://people.mozilla.org/~nchen/bug810821-2-fennec-20.0a1.apk

Comment 20

[rclement42]

That works great!!

No more deselection when hovering above the keyboard!

Comment 21

[Aaron Train [:aaronmt]]

Let's get a fix in there!

Comment 22

[(inactive) Jim Chen [:jchen] [:darchons]]

(In reply to rclement42 from comment #20)
> That works great!!
> 
> No more deselection when hovering above the keyboard!

Great! Do you mind testing again with this Nightly? people.mozilla.org/~nchen/bug810821-3-fennec-20.0a1.apk  The last one had some unintended side effects, and I want to make sure that without those side effects, this new Nightly still fixes the bug. Thanks!

Comment 23

[rclement42]

Yes, it's still fixed!

Comment 24

[(inactive) Jim Chen [:jchen] [:darchons]]

Attachment: Only change focus if awesomebar text field responded to key press (v1)

If the awesomebar text field does not respond to keypress events (i.e. when the selection doesn't change), we shouldn't try to give it focus. When we do give it focus, the patch also restores the whole selection, not just the cursor point.

Comment 25

[Chris Peterson [:cpeterson]]

Comment on attachment 691842 [details] [diff] [review]
Only change focus if awesomebar text field responded to key press (v1)

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

LGTM with these changes:

::: mobile/android/base/AwesomeBar.java
@@ +451,2 @@
>                  selStart = mText.getSelectionStart();
>                  selEnd = mText.getSelectionEnd();

1. Should we be checking "start changed *OR* end changed", not "AND"?

2. I think you should consolidate all these mText.getSelectionStart/End() calls. The code will be shorter and the intent will be clearer with something like:

  if (prevSelStart != curSelStart || prevSelEnd != curSelEnd) {

Comment 26

[(inactive) Jim Chen [:jchen] [:darchons]]

Attachment: Only change focus if awesomebar text field responded to key press (v1.1)

(In reply to Chris Peterson (:cpeterson) from comment #25)
> Comment on attachment 691842 [details] [diff] [review]
> Only change focus if awesomebar text field responded to key press (v1)
> 
> Review of attachment 691842 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> LGTM with these changes:
> 
> ::: mobile/android/base/AwesomeBar.java
> @@ +451,2 @@
> >                  selStart = mText.getSelectionStart();
> >                  selEnd = mText.getSelectionEnd();
> 
> 1. Should we be checking "start changed *OR* end changed", not "AND"?

Good catch!

> 2. I think you should consolidate all these mText.getSelectionStart/End()
> calls. The code will be shorter and the intent will be clearer with
> something like:
> 
>   if (prevSelStart != curSelStart || prevSelEnd != curSelEnd) {

Done. Carried over r+.

https://hg.mozilla.org/integration/mozilla-inbound/rev/e3998db26591

Comment 27

[(inactive) Jim Chen [:jchen] [:darchons]]

Leaving open for nominate uplifting

Comment 28

[(inactive) Jim Chen [:jchen] [:darchons]]

Comment on attachment 692360 [details] [diff] [review]
Only change focus if awesomebar text field responded to key press (v1.1)

[Approval Request Comment]
Bug caused by (feature/regressing bug #): 
User impact if declined: Very unpleasant user experience on the Galaxy Note II; awesomebar deselects when using the pen
Testing completed (on m-c, etc.): Locally; through bug reporter
Risk to taking this patch (and alternatives if risky): Very small; the code only runs in specific situations, and the new code preserves the previous behavior as much as possible while fixing the bug
String or UUID changes made by this patch: None

Comment 29

[(inactive) Jim Chen [:jchen] [:darchons]]

Comment on attachment 692360 [details] [diff] [review]
Only change focus if awesomebar text field responded to key press (v1.1)

[Approval Request Comment]
Bug caused by (feature/regressing bug #): 
User impact if declined: Very unpleasant user experience on the Galaxy Note II; awesomebar deselects when using the pen
Testing completed (on m-c, etc.): Locally; through bug reporter
Risk to taking this patch (and alternatives if risky): Very small; the code only runs in specific situations, and the new code preserves the previous behavior as much as possible while fixing the bug
String or UUID changes made by this patch: None

Comment 30

[Alex Keybl [:akeybl]]

Approving for Aurora/Beta given this is a low risk fix. We should do specific verification around awesomebar use cases (text selection, typing, etc.) on Aurora before we go to build with Beta 5 Tuesday.

Comment 31

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/e3998db26591

Comment 32

[Ed Morley [:emorley]]

Was landed on aurora by Jim:
https://hg.mozilla.org/releases/mozilla-aurora/rev/aef21af0f7ae

However, backed out for landing on a closed tree (treestatus.m.o is partially down at the moment, and the tree closure hook defaults to open [soon to be changed in bug 758888]).

https://hg.mozilla.org/releases/mozilla-aurora/rev/ef6e99740705

Comment 33

[(inactive) Jim Chen [:jchen] [:darchons]]

Landed safely this time :)

https://hg.mozilla.org/releases/mozilla-aurora/rev/3bfaad61fd5f
https://hg.mozilla.org/releases/mozilla-beta/rev/7d9672bd37a5

Comment 34

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-b2g18/rev/7d9672bd37a5