1342788 - Autocompleted domains do a search using Gboard

Status: VERIFIED FIXED

(Firefox for iOS :: Browser, defect, P1)

Description

[Brian Nicholson (:bnicholson)]

Bug report from mfinkle: https://twitter.com/mfinkle/status/835630006374105097

Switch to the Gboard keyboard. Type "wiki", which will autocomplete to "wikipedia.org". Hit Go. This will do a Google search instead of going to the autocompleted domain.

Comment 1

[Brian Nicholson (:bnicholson)]

Google search -> search with the default search engine

Comment 3

[Maurya Talisetti]

Attachment: Link to Github pull-request: https://github.com/mozilla-mobile/firefox-ios/pull/2563

Brian, hitting the "Go" button on a Gboard is causing the textField(_:shouldChangeCharactersIn:replacementString:) function to be called. It's basically trying to say that no new text has been entered, but I still am calling this function. I couldn't find any documentation that describes this behavior with third-party keyboards. This behavior doesn't happen for the system keyboard.

How does this matter to us? Well, I just wanted to be sure that we are not explicitly creating a callback to this function when the "Go" button is tapped on a third-party keyboard. If we are, then I want to verify that flow for bugs as well. I couldn't find any reference in the code to such a callback. So, just curious if you might have any thoughts on this.

Comment 4

[Brian Nicholson (:bnicholson)]

Comment on attachment 8851338 [details] [review]
Link to Github pull-request: https://github.com/mozilla-mobile/firefox-ios/pull/2563

This is fragile code, so please test extensively! I'll land once you can confirm you've tested the steps mentioned in the pull request (plus anything else you can think of).

Comment 5

[Catalin Suciu [:csuciu]]

I've tested this patch in a local build and the bug is no longer reproducible, I've also tested the scenarios mentioned in the pull request using the standard keyboard and the behavior is as expected.

Farhan,

Comment 6

[Catalin Suciu [:csuciu]]

Hit save to soon :)

Maurya, Farhan, can we land this patch?

Comment 7

[Farhan Patel [:farhan]]

Im cool with merging this once its rebased and the tests are re-run :)

Comment 9

[Justin D'Arcangelo [:justindarc]]

Need to land this.

Comment 10

[Maurya Talisetti]

Justin, this has been rebased. But, unfortunately I'm not able to test on a real device as I don't have the ability to install onto a real device.

Comment 11

[Maurya Talisetti]

Attachment: Link to Github pull-request: https://github.com/mozilla-mobile/firefox-ios/pull/2563

Actually I managed to find a way to install to a device and test. Repeated the tests mentioned in the PR and it looks good.

Comment 12

[Farhan Patel [:farhan]]

Comment on attachment 8882769 [details] [review]
Link to Github pull-request: https://github.com/mozilla-mobile/firefox-ios/pull/2563

There is a small change that needs to be made that I will land in a follow up

Comment 13

[Farhan Patel [:farhan]]

master 

https://github.com/mozilla-mobile/firefox-ios/commit/29ab35502f71d5461c99de1d21c03e69548852b9
https://github.com/mozilla-mobile/firefox-ios/commit/6ad4a2de5247c32bdba370124add2887ba596d73

Comment 14

[Maurya Talisetti]

(In reply to Farhan Patel [:farhan] from comment #12)
> Comment on attachment 8882769 [details] [review]
> Link to Github pull-request:
> https://github.com/mozilla-mobile/firefox-ios/pull/2563
> 
> There is a small change that needs to be made that I will land in a follow up

Yeah I've noticed there has been a push to land this feature. Unfortunately, I still don't have my physical mac with me and have been using a remote mac which doesn't allow me to install the app onto my phone. The Gboard can't be installed on the simulator, so I need to test on the phone.

Comment 15

[Catalin Suciu [:csuciu]]

Verifying as fixed in master e14617f7e

Comment 16

[Stefan Arentz | :st3fan | ⏰ EST | he/him]

Uplifted to v8.x

Comment 17

[Stefan Arentz | :st3fan | ⏰ EST | he/him]

Reverted this patch in v8.x

Comment 18

[Farhan Patel [:farhan]]

fixed in
https://github.com/mozilla-mobile/firefox-ios/commit/b7d2697641fda38ee9851650f01fb62ad207adb5

Comment 19

[Simon Basca [:SimonB]]

Verifying as fix on master and beta 8.2(5120).