1557302 - Extra space is not being generated if the user manually inputs the full @ search engine name (ex @google) and presses the enter key inside the URL bar

Status: RESOLVED FIXED

(Firefox :: Address Bar, defect, P3)

Description

[Emil Ghitta, QA [:emilghitta]]

Attachment: issue.gif

Affected versions

• Firefox 68.0b7 (BuildId:20190603181408).
• Firefox 69.0a1 (BuildId:20190605215957)

Unaffected versions

• Firefox 60.7.0 esr (BuildId:20190514153425)
• Firefox 67.0.1 (BuildId:20190529130856)

Affected platforms

• Windows 10 64bit
• Ubuntu 18.04 64bit
• macOS 10.14

Steps to reproduce

1. Launch Firefox.
2. Type @google inside the url input bar.
3. Press the enter key

Expected result

• An extra space is generated after the @google string and the user can enter additional information inside the url bar.

Actual result

• No extra space is generated and the user has to manually press the spacebar button.

Regression range

• Pushlog:
  https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=38a326f813f6b1fd1400d215730e0105f62eb9c5&tochange=725cc368d14453980d6e0b112edc263a12463ffe
• Potential regressor: Bug 1548031

Additional Information

• I have attached a screencast for this issue

Comment 1

[Emil Ghitta, QA [:emilghitta]]

Hi Marco,

It seems that the patch for Bug 1548031 may have caused this regression.

Can you please have a look?

Thanks!

Comment 2

[Marco Bonardo [:mak]]

It looks like an edge case I did not handle in bug 1555277.

Comment 3

[Marco Bonardo [:mak]]

I think the problem is that in unifiedcomplete we exit earlier in _matchSearchEngineAlias, thus we don't reach _matchSearchEngineTokenAliasForAutofill. As a consequence the unifiecomplete provider doesn't fill any value in match.autofill.

I wonder where's the best point to address this, probably _matchSearchEngineAlias should not handle this and leave it to _matchSearchEngineTokenAliasForAutofill. There is also an edge case we don't handle, having 2 aliases where one contains the other, like @google and @google-two, we should not autofill the latter until - is typed, but aliases are not sorted, we just pick the first engine that matches. Changing the current code may thus autofill the wrong one, while until now we were bailing out. This must be taken into account.

Comment 4

[Marco Bonardo [:mak]]

(In reply to Marco Bonardo [::mak] from comment #3)

There is also an edge case we don't handle, having 2 aliases where one contains the other, like @google and @google-two, we should not autofill the latter until - is typed, but aliases are not sorted, we just pick the first engine that matches.

Rethinking about this, the user has control over the order of engines, and we get the list of engines from the search service in that order, so doing internal sorting may break expectations, we can just rely on the user's choice.

Comment 5

[Marco Bonardo [:mak]]

Attachment: * Bug 1557302 - Enter on a fully typed @alias should move the caret at the end. r=adw

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/0cb2bef9b251
* Bug 1557302 - Enter on a fully typed @alias should move the caret at the end. r=adw

Comment 7

[Stefan Hindli [:stefan_hindli]]

https://hg.mozilla.org/mozilla-central/rev/0cb2bef9b251

Comment 8

[Ryan VanderMeulen [:RyanVM]]

Please nominate this for Beta approval when you get a chance. Thanks for including a new test too!

Comment 9

[Marco Bonardo [:mak]]

Comment on attachment 9071940 [details]

• Bug 1557302 - Enter on a fully typed @alias should move the caret at the end. r=adw

Beta/Release Uplift Approval Request

• User impact if declined: typing an @alias and pressing Enter doesn't move the cursor to the end
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: No
• Needs manual test from QE?: Yes
• If yes, steps to reproduce: type @google and press Enter, the curso should move 1 space after @google, allowing to immediately type a search string
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Simplifies the patch previously landed in Bug 1555277, trivial change
• String changes made/needed:

Comment 10

[Julien Cristau [:jcristau] (back April 22)]

Comment on attachment 9071940 [details]

• Bug 1557302 - Enter on a fully typed @alias should move the caret at the end. r=adw

approved for 68.0b11

Comment 11

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/releases/mozilla-beta/rev/291429454e74