Closed Bug 1541929 Opened 7 months ago Closed 6 months ago

On undo/Ctrl+Z, autofill completes the URL without a selection and moves the caret to the end

Categories

(Firefox :: Address Bar, defect, P2)

defect
Points:
3

Tracking

()

RESOLVED FIXED
Firefox 68
Iteration:
68.3 - Apr 15 - 28
Tracking Status
firefox68 --- fixed

People

(Reporter: mak, Assigned: adw)

References

Details

Attachments

(1 file)

While reproducing another bug, I hit this case.
I time a few random chars in the urlbar, starting from a, like "asdfwqekj", I press ctrl+Z, it changes to "asrock.com/"... At first I was surprised, then I noticed in my profile "as" autofills to asrock.com.
The big problem is that there's no selection, the site is just autofilled in place. I think we should not autofill in this case, but the old urlbar does autofill, with removable selection, that is already a better solution than what we have now.

Didn't really mean to transform adw in a bug!

Alias: adw

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

Didn't really mean to transform adw in a bug!

That's deep, man.

Priority: P3 → P2
Assignee: nobody → adw
Status: NEW → ASSIGNED

nsAutoCompleteController doesn't complete the default index when there's a text selection. iow selectionStart must == selectionEnd for autofill to happen due to processing the first result. UrlbarInput doesn't have that restriction, which is why this happens in quantumbar but not awesomebar.

Specifically, nsAutoCompleteController returns here: https://searchfox.org/mozilla-central/rev/dd7e27f4a805e4115d0dbee70e1220b23b23c567/toolkit/components/autocomplete/nsAutoCompleteController.cpp#1475

And UrlbarInput does the autofill in this bug here: https://searchfox.org/mozilla-central/rev/dd7e27f4a805e4115d0dbee70e1220b23b23c567/browser/components/urlbar/UrlbarInput.jsm#488

I'd like to fix this bug by bring quantumbar to parity with awesomebar as I just described, and then file another bug for considering not autofilling at all when undoing. They're two separate tasks that we should consider separately imo. (We might be able to use editor's nsITransactionListener to listen for undo and prohibit autofill: https://searchfox.org/mozilla-central/rev/dd7e27f4a805e4115d0dbee70e1220b23b23c567/editor/txmgr/nsITransactionListener.idl#55 )

We need to handle autofilling the first result separately from autofilling results in general (which happens in UrlbarInput.setValueFromResult), so add a new UrlbarInput.autofillFirstResult method. The controller calls it instead of setValueFromResult. I ported the logic from nsAutoCompleteController, as described in the bug.

Other changes are related to the new test for this.

As part of this work, I was interested in learning how awesomebar handles browser_autoFill_typed.js, so I added it to the legacy tests, with a small tweak in the test for awesomebar.

Iteration: --- → 68.3 - Apr 15 - 28
Pushed by dwillcoxon@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/ea02b41a2b1d
Don't autofill the first result in some cases. r=mak
Status: ASSIGNED → RESOLVED
Closed: 6 months ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 68
See Also: → 1545559

I filed bug 1545559 for specifically not autofilling on undo. I'll update this bug's summary so it's clear what we ended up fixing.

Summary: Autofill completes the url on ctrl+z → On undo/Ctrl+Z, autofill completes the URL without a selection and moves the caret to the end
Regressions: 1544996
You need to log in before you can comment on or make changes to this bug.