Closed Bug 1344464 Opened 3 years ago Closed 3 years ago

locationbar/navigationbar: Tapping backspace deletes suggested part of url, second backspace deletes two characters or re-adds it [xperia keyboard]

Categories

(Firefox for Android :: Keyboards and IME, defect, P1, major)

All
Android
defect

Tracking

()

VERIFIED FIXED
Firefox 55
Tracking Status
fennec 53+ ---
firefox51 --- unaffected
firefox52 --- unaffected
firefox-esr52 --- unaffected
firefox53 ? fixed
firefox54 --- fixed
firefox55 --- fixed

People

(Reporter: aryx, Assigned: JanH)

References

Details

(Keywords: regression)

Attachments

(2 files)

Ok, the issue here seems to be that Firefox wants to delete two characters after the second backspace. If there is only character in the location bar, this fails and the autofill applies again.
Summary: locationbar/navigationbar: Tapping backspace deletes suggested part of url, second backspace returns it → locationbar/navigationbar: Tapping backspace deletes suggested part of url, second backspace deletes two characters or re-adds it
Latest Firefox for Android 54.0a1 on Android 6.0.1 (Sony Xperia Z3 Compact)

Tapping backspace after having typed one letter in the location bar deletes the suggested part of the url (suggestion sometimes doesn't show up on first try), a second tap on backspace will add it again, so you can't fix typos. If there are two or more characters in the location bar, two will be deleted on that second backspace.

It should delete the previous character only that one.

This is a regression between 20161130 and 20161201. Commits: https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=13736e2db6eb94b02dd28cc88f2943b8109aa374&tochange=cd4cdcc9ad6c45dad8b8d8c0d40e459db2bca8a1

Regression from bug 1320449 or bug 1320603?
Flags: needinfo?(jh+bugzilla)
Which keyboard are you using?

Our options probably are:
- find some workaround
- blacklist the affected IME
- entirely disable input suggestions again
Flags: needinfo?(jh+bugzilla)
Blocks: 1320449
tracking-fennec: --- → ?
Component: Awesomescreen → Keyboards and IME
OS: Unspecified → Android
Hardware: Unspecified → All
> Which keyboard are you using?
The default one. Translated from German: "International Keyboard - Xperia(TM) Keyboard" (the available keyboards are either Xperia ones or Voice input).
Works as expected with latest Nightly on a Nexus S with Android 4.1.2
Summary: locationbar/navigationbar: Tapping backspace deletes suggested part of url, second backspace deletes two characters or re-adds it → locationbar/navigationbar: Tapping backspace deletes suggested part of url, second backspace deletes two characters or re-adds it [xperia keyboard]
I've obtained the Xperia keyboard from XDA (https://forum.xda-developers.com/android/apps-games/app-sony-xperia-keyboard-phones-t3346736) and I can indeed reproduce it.

It seems that the relevant bit of autocomplete logic works by intercepting setComposingText (https://dxr.mozilla.org/mozilla-central/rev/80c06df83395314697d464f88f8daa98bf05465c/mobile/android/base/java/org/mozilla/gecko/toolbar/ToolbarEditText.java#481).

Let's assume we've typed wiki, which autocompleted to wiki<pedia.org>.
When pressing backspace, setComposingText gets called with "wik" for the text parameter. We detect that the composing text is getting shorter, remove the autocomplete and then return *false* to prevent the keyboard's changes from having any actual effect on the URL bar.
Other keyboards (okay, at least Swiftkey - I don't have that many keyboards installed and the Google keyboard doesn't offer input suggestions in URL input mode and uses deleteSurroundingText instead) handle this fine, so the next time backspace is pressed they're calling setComposingText("wik") again.

The Xperia keyboard however seems to ignore setComposingText returning false and assumes it successfully deleted that letter, so the next time the user presses backspace, it sends setComposingText("wi"), thereby apparently deleting two letters in one go.
If there was just one user-typed letter, it oscillates between "" and "w" and causes the behaviour you've observed above.

As a workaround, it seems that restarting the input before returning false from setComposingText() works fine.
Assignee: nobody → jh+bugzilla
Has Regression Range: --- → yes
Has STR: --- → yes
Priority: -- → P1
Comment on attachment 8843631 [details]
Bug 1344464 - Part 0 - Remove unneeded code.

https://reviewboard.mozilla.org/r/117252/#review119308
Attachment #8843631 - Flags: review?(nchen) → review+
Comment on attachment 8843632 [details]
Bug 1344464 - Part 1 - Restart input after removing autocomplete text for keyboards that require it.

https://reviewboard.mozilla.org/r/117254/#review119310
Attachment #8843632 - Flags: review?(nchen) → review+
Pushed by mozilla@buttercookie.de:
https://hg.mozilla.org/integration/autoland/rev/08e4e7a7af7d
Part 0 - Remove unneeded code. r=jchen
https://hg.mozilla.org/integration/autoland/rev/960852d0618a
Part 1 - Restart input after removing autocomplete text for keyboards that require it. r=jchen
https://hg.mozilla.org/mozilla-central/rev/08e4e7a7af7d
https://hg.mozilla.org/mozilla-central/rev/960852d0618a
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 55
Can you verify that this is working as expected and doesn't have any other unwanted side effects?
Flags: needinfo?(aryx.bugmail)
tracking-fennec: ? → 53+
Please request Aurora/Beta approval on this when you're ready to do so.
Flags: needinfo?(jh+bugzilla)
Flags: in-testsuite+
Flags: in-testsuite+
Works now as expected with 55.0a1 20170308 on the Xperia Z3C.
Status: RESOLVED → VERIFIED
Flags: needinfo?(aryx.bugmail)
Comment on attachment 8843631 [details]
Bug 1344464 - Part 0 - Remove unneeded code.

see Part 1 for rest
[Is the change risky?]: No.
[Why is the change risky/not risky?]: Removed imports in ToolbarEditText.java aren't necessary in Beta codebase either and InputMethods.isGestureKeyboard() was likewise already disused at that time.
Attachment #8843631 - Flags: approval-mozilla-beta?
Attachment #8843631 - Flags: approval-mozilla-aurora?
Comment on attachment 8843632 [details]
Bug 1344464 - Part 1 - Restart input after removing autocomplete text for keyboards that require it.

Approval Request Comment
[Feature/Bug causing the regression]: Bug 1320449
[User impact if declined]: Strange behaviour when deleting text on the URL bar in conjunction with URL autocompletion.
[Is this code covered by automated tests?]: No.
[Has the fix been verified in Nightly?]: Yes.
[Needs manual test from QE? If yes, steps to reproduce]: No.
[List of other uplifts needed for the feature/fix]: Part 0 would be nice, although not absolutely necessary.
[Is the change risky?]: No.
[Why is the change risky/not risky?]: Restarting input is already used as a workaround to force the keyboard noticing our actions in similar circumstances. Additionally, this code is restricted to a specific keyboard app only.
[String changes made/needed]: none
Flags: needinfo?(jh+bugzilla)
Attachment #8843632 - Flags: approval-mozilla-beta?
Attachment #8843632 - Flags: approval-mozilla-aurora?
Comment on attachment 8843631 [details]
Bug 1344464 - Part 0 - Remove unneeded code.

Fix a deleting text on the URL bar issue with URL autocompletion and was verified. Aurora54+.
Attachment #8843631 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Attachment #8843632 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Comment on attachment 8843632 [details]
Bug 1344464 - Part 1 - Restart input after removing autocomplete text for keyboards that require it.

Recent regression in 53, fix has been verified on 54, Beta53+
Attachment #8843632 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Attachment #8843631 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Hi, 

This bug is reproducible with Huawei MediaPad M2 (Android 5.1.1) and keyboard Swype- Huawei Swype on all branches, 52 - 55. I type wiki, one backspace deleted "pedia.org", and the second backspace deleted "ki".
Same for Samsung Galaxy Note 4 (Android 5.0.1) with SWYPE keyboard.
With Android keyboard (AOSP) this issue is not reproducible.
Please create a new bug for this issue. That is a different issue because it already affects Firefox 52.
Flags: needinfo?(sorina.florean)
See Also: → 1348799
Filed Bug 1348799 - Second backspace is deleting two characters instead of one.
Flags: needinfo?(sorina.florean)
You need to log in before you can comment on or make changes to this bug.