Closed
Bug 1333345
Opened 8 years ago
Closed 5 years ago
Ctrl+Enter in address bar will open autocomplete-suggested site name instead of the actual text entered + ".com"
Categories
(Firefox :: Address Bar, defect, P3)
Firefox
Address Bar
Tracking
()
People
(Reporter: cpeterson, Assigned: cpeterson)
References
Details
(Whiteboard: [fxsearch])
Attachments
(4 files)
STR:
1. In your address bar, enter "getfirefox" and CTRL+ENTER to open getfirefox.com.
2. In your address bar, enter "ge" and CTRL+ENTER to open ge.com.
RESULT:
The address bar will autocomplete "ge" to "getfirefox.com" and CTRL+ENTER will incorrectly open getfirefox.com instead of ge.com. Chrome and Edge correctly open ge.com with this same STR.
Entering "ge " with a space and then backspacing to dismiss the autocomplete suggestion, followed by CTRL+ENTER, will correctly open ge.com.
This does not appear to be a regression. I can reproduce on Windows and Mac on ESR 45 and later.
|
||
Comment 1•8 years ago
|
||
You are right, CTRL+ENTER should act on the original typed string.
Priority: -- → P3
Whiteboard: [fxsearch]
|
Assignee | |
Comment 2•8 years ago
|
||
I have some patches I will submit shortly. Here is a mostly green Try run:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=9848e86a2e76c797c41ea45f0953c327fb4256d3
Assignee: nobody → cpeterson
Summary: CTRL+ENTER in address bar will open autocomplete-suggested site name instead of the actual text entered + ".com" → Ctrl+Enter in address bar will open autocomplete-suggested site name instead of the actual text entered + ".com"
| Comment hidden (mozreview-request) |
| Comment hidden (mozreview-request) |
| Comment hidden (mozreview-request) |
| Comment hidden (mozreview-request) |
|
|
||
Comment 7•8 years ago
|
||
Adding a dependency on bug 1337682, this will likely need a small unbitrot.
Depends on: 1337682
|
|
||
Comment 8•8 years ago
|
||
| mozreview-review | ||
Comment on attachment 8841844 [details]
Bug 1333345 - Part 1: Add urlbar tests for Accel+Enter and Accel+Shift+Enter adding .com and .org suffixes to search string.
https://reviewboard.mozilla.org/r/115928/#review118646
Unfortunately this needs an unbitrot, on the other side, this is partially done already
Attachment #8841844 -
Flags: review?(mak77)
|
|
||
Comment 9•8 years ago
|
||
| mozreview-review | ||
Comment on attachment 8841846 [details]
Bug 1333345 - Part 3: Consolidate maybeCanonizeURL/handleEnter calls into handleEnterNow.
https://reviewboard.mozilla.org/r/115932/#review118648
::: browser/base/content/urlbarBindings.xml:693
(Diff revision 1)
> + <method name="handleEnterNow">
> + <parameter name="aTriggeringEvent"/>
> + <parameter name="aURL"/>
> + <body><![CDATA[
> + this.maybeCanonizeURL(aTriggeringEvent, aURL);
> + return this.mController.handleEnter(false, aTriggeringEvent);
This also needs an unbitrot, it should probably also clear overrideValue, based on the new code
Attachment #8841846 -
Flags: review?(mak77)
|
|
||
Comment 10•8 years ago
|
||
| mozreview-review | ||
Comment on attachment 8841845 [details]
Bug 1333345 - Part 2: Add test to demonstrate buggy keyboard shortcut behavior in address bar.
https://reviewboard.mozilla.org/r/115930/#review118650
Attachment #8841845 -
Flags: review?(mak77) → review+
|
|
||
Comment 11•8 years ago
|
||
| mozreview-review | ||
Comment on attachment 8841847 [details]
Bug 1333345 - Part 4: Fix address bar keyboard shortcuts to add domain name suffix instead of loading the domain suggested by autocomplete.
https://reviewboard.mozilla.org/r/115934/#review118662
::: browser/base/content/urlbarBindings.xml:696
(Diff revision 1)
> - this.maybeCanonizeURL(aTriggeringEvent, aURL);
> + // If the default (first) result is selected, then it (this.
> + // mController.searchString) might be a URL, a search string, or
> + // a string that should be expanded to a URL. If any result other than
> + // the first is selected, then it must have been manually selected by
> + // the human. We let this explicit choice (this.value) be used as-is.
> + if (this.popup.selectedIndex == 0) {
I'm not sure regarding this, the user may have selected a search suggestions and may want to complete it to an url.
for example, I may type "por", one of the search suggestions may be "porcupine", I could select it and shift+Enter to visit "www.porcupine.com".
I think we should rather check if the selected entry is an autofill entry, and only in such a case use searchString.
You may have to change "_isFirstResultHeuristic" to a method like "_firstResultIs(style)" and then replace "_isFirstResultHeuristic" with "_firstResultIs("heuristic")". Then you could use "this.popup.selectedIndex == 0 && _firstResultIs("autofill")" to decide which string to use.
Attachment #8841847 -
Flags: review?(mak77)
|
|
Assignee | |
Comment 12•8 years ago
|
||
(In reply to Marco Bonardo [::mak] from comment #11)
> > - this.maybeCanonizeURL(aTriggeringEvent, aURL);
> > + // If the default (first) result is selected, then it (this.
> > + // mController.searchString) might be a URL, a search string, or
> > + // a string that should be expanded to a URL. If any result other than
> > + // the first is selected, then it must have been manually selected by
> > + // the human. We let this explicit choice (this.value) be used as-is.
> > + if (this.popup.selectedIndex == 0) {
>
> I'm not sure regarding this, the user may have selected a search suggestions
> and may want to complete it to an url.
> for example, I may type "por", one of the search suggestions may be
> "porcupine", I could select it and shift+Enter to visit "www.porcupine.com".
>
> I think we should rather check if the selected entry is an autofill entry,
> and only in such a case use searchString.
> You may have to change "_isFirstResultHeuristic" to a method like
> "_firstResultIs(style)" and then replace "_isFirstResultHeuristic" with
> "_firstResultIs("heuristic")". Then you could use "this.popup.selectedIndex
> == 0 && _firstResultIs("autofill")" to decide which string to use.
Thanks. I will look into these cases.
|
|
Assignee | |
Comment 13•8 years ago
|
||
(In reply to Marco Bonardo [::mak] from comment #11)
> I'm not sure regarding this, the user may have selected a search suggestions
> and may want to complete it to an url.
> for example, I may type "por", one of the search suggestions may be
> "porcupine", I could select it and shift+Enter to visit "www.porcupine.com".
Given input string "por" and word suggestion "porcupine" (or site or tab suggestion "porcupine.com"), I think the behavior we want is:
* Enter of the first entry will either search for word suggestion "porcupine" or open site suggestion "porcupine.com". (Current behavior)
* Ctrl+Enter of the first entry will open "www.por.com", even if the first entry is the suggestion "porcupine.com". (New behavior like Chrome, implemented by my patches here)
* Enter of a word suggestion like "porcupine" lower in the list will search for "porcupine". (Current behavior)
* Ctrl+Enter of a word suggestion like "porcupine" lower in the list will open "www.porcupine.com". (New behavior like Chrome, to be implemented)
* Enter of a *tab* suggestion lower in the list will switch to that other tab. (Current behavior)
* Ctrl+Enter of a *tab* suggestion lower in the list will open that URL in the current tab. (Current behavior)
Rationale:
AFAICT, Firefox never places a *word or phrase* suggestion like "porcupine" into the first/default entry. The first entry is always the input string "por" or a *site* suggestion like "porcupine.com" if you have visited that site before. OTOH, Chrome does place the word suggestion "porcupine" into the first entry, but Ctrl+Enter will expand the input string "por" to "www.por.com" instead of the word suggestion "porcupine" to "www.porcupine.com".
When you say "I could select it", do you mean using the down arrow or tab key to highlight a word suggestion that is not the first entry? In this case, Ctrl+Enter in Firefox currently ignores the Ctrl and just searches for "porcupine". OTOH, Chrome does what you describe and expands the word suggestion to "www.porcupine.com". Given that you describe this use case as something a user might want to do and it is Chrome's behavior, Firefox should probably do this.
Note that the suggestion list can suggest another open tab. In this case, the list entry will have tab icon and Enter will switch focus to that other tab instead of searching for the input string. If you select the tab suggestion with Ctrl+Enter or Shift+Enter, then Firefox will open that URL in the current tab instead of switching to the other tab. We should not change this current behavior. This is not an issue for Chrome because it doesn't switch to other open tabs from the suggestion list. It will always open the suggested URL in the current tab.
|
|
||
Comment 14•8 years ago
|
||
(In reply to Chris Peterson [:cpeterson] from comment #13)
> Given input string "por" and word suggestion "porcupine" (or site or tab
> suggestion "porcupine.com"), I think the behavior we want is:
>
> * Enter of the first entry will either search for word suggestion
> "porcupine" or open site suggestion "porcupine.com". (Current behavior)
> * Ctrl+Enter of the first entry will open "www.por.com", even if the first
> entry is the suggestion "porcupine.com". (New behavior like Chrome,
> implemented by my patches here)
Right. If it's an "autofill" suggestion, it will always be a domain, in such a case canonization makes only sense on the original typed string.
> * Enter of a word suggestion like "porcupine" lower in the list will search
> for "porcupine". (Current behavior)
> * Ctrl+Enter of a word suggestion like "porcupine" lower in the list will
> open "www.porcupine.com". (New behavior like Chrome, to be implemented)
Yep, this atm can only happen for search suggestions I think. Basically the completed word is not a domain, so it can be completed to a host. There are edge cases here, like the word may contain spaces or chars that are not allowed in a host, but I think we don't want to go to deep into covering all of these honestly, because:
1. it's an undiscoverable and thus rarely used feature
2. the users of this feature are more likely to understand the technical limitations of it (so that "my cat is fat" is unlikely to work as a domain)
> * Enter of a *tab* suggestion lower in the list will switch to that other
> tab. (Current behavior)
> * Ctrl+Enter of a *tab* suggestion lower in the list will open that URL in
> the current tab. (Current behavior)
Right.
> When you say "I could select it", do you mean using the down arrow or tab
> key to highlight a word suggestion that is not the first entry? In this
> case, Ctrl+Enter in Firefox currently ignores the Ctrl and just searches for
> "porcupine".
Yes, I mean selecting it through a way that replaces the contents of the input field. So either by down or (currently) tab.
OTOH, Chrome does what you describe and expands the word
> suggestion to "www.porcupine.com". Given that you describe this use case as
> something a user might want to do and it is Chrome's behavior, Firefox
> should probably do this.
I didn't know Chrome was doing it, but makes sense :)
|
|
||
Comment 15•5 years ago
|
||
This WFM in nightly
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → WORKSFORME
|
|
Assignee | |
Comment 16•5 years ago
|
||
This fix makes me very happy! :)
Out of curiosity, I tried to bisect the fix for this bug. I think it was fixed in Firefox 56 by bug 448486. mozregression could bisect any further on these old builds.
Status: RESOLVED → VERIFIED
Depends on: 448486
You need to log in
before you can comment on or make changes to this bug.
Description
•