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

RESOLVED FIXED in Firefox 68

Status

()

defect
P3
minor
RESOLVED FIXED
2 months ago
Last month

People

(Reporter: emilghitta, Assigned: mak)

Tracking

(Regression, {regression})

Trunk
Firefox 69
Points:
2
Dependency tree / graph
Bug Flags:
in-testsuite +
qe-verify +

Firefox Tracking Flags

(firefox-esr60 unaffected, firefox67 unaffected, firefox67.0.1 unaffected, firefox68 fixed, firefox69 fixed)

Details

Attachments

(2 attachments)

Posted image 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

Additional Information

  • I have attached a screencast for this issue

Hi Marco,

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

Can you please have a look?

Thanks!

Flags: needinfo?(mak77)
Has Regression Range: --- → yes
Has STR: --- → yes

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

Points: --- → 2
Flags: needinfo?(mak77)
Priority: -- → P3
Regressed by: 1555277
Blocks: 1555277
No longer regressed by: 1555277
Assignee: nobody → mak77
Status: NEW → ASSIGNED

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.

(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.

Attachment #9071940 - Attachment description: * Bug 1557302 - Autofill an @alias even if it's fully typed. r=adw → * Bug 1557302 - Enter on a fully typed @alias should move the caret at the end. r=adw
Pushed by mak77@bonardo.net:
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
Status: ASSIGNED → RESOLVED
Closed: 2 months ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 69

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

Flags: needinfo?(mak77)
Flags: in-testsuite+

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:
Attachment #9071940 - Flags: approval-mozilla-beta?
Flags: qe-verify+
Flags: needinfo?(mak77)

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

Attachment #9071940 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
QA Whiteboard: [qa-triaged]
Iteration: --- → 69.3 - Jun 10 - 23
You need to log in before you can comment on or make changes to this bug.