Location bar loads address including >> instead of just the replaced address

VERIFIED FIXED in Firefox 35

Status

()

Firefox
Location Bar
VERIFIED FIXED
3 years ago
3 years ago

People

(Reporter: Gijs, Assigned: mak)

Tracking

Trunk
Firefox 35
Points:
5
Dependency tree / graph
Bug Flags:
firefox-backlog +
in-testsuite +
qe-verify +

Firefox Tracking Flags

(firefox35 verified)

Details

Attachments

(1 attachment)

(Reporter)

Description

3 years ago
STR:

0. Clean profile
1. In the first tab that opens (https://www.mozilla.org/en-US/firefox/nightly/firstrun/), stop the load, and clear the location bar
2. Type "www.g"
3. After autocomplete appears, click the "go" arrow with the mouse or hit enter (both seem to work/break similarly)


ER:
Autocomplete to www.google.com

AR:
Loads a search page for "www.g >> google.com"

Updated

3 years ago
Flags: qe-verify+
Flags: firefox-backlog+

Updated

3 years ago
Points: --- → 5
I may work on that but before taking it I have some questions.

Where does that functionality come from and what's the reason for that? It looks pretty ugly but I guess it was discussed somewhere and there is some reason we do it (personally, I like what we have now much more - just add the rest of the text we suggest without this fancy ">>". It's short and clear)
Flags: needinfo?(gijskruitbosch+bugs)
Comment hidden (obsolete)
(Assignee)

Comment 3

3 years ago
in urlbar we should =*_NEVER_*= see ">>" completion. unfortunately in some edge-cases autocompleteController still tries to do that, to avoid that the search provider(UnifiedComplete.js) tries to never return an autoFill result that doesn't match the beginning of the current code.
(Assignee)

Comment 4

3 years ago
s/code/search string/
I'll be happy to dive into the search/suggestion code. I'm also happy that seeing ">>" at all is a bug as it looks really weird. So I guess this is not a feature but a regression, right?

I did a quick code search. Is the code responsible for that here: http://hg.mozilla.org/mozilla-central/file/e58842c764dd/toolkit/components/autocomplete/nsAutoCompleteController.cpp#l1637?

I'd welcome any more information about this bug. Please keep in mind that I'll see this code for the first time so I don't really know its mechanics.
Flags: needinfo?(mak77)
(Reporter)

Comment 6

3 years ago
(I've hidden my earlier comment, which is clearly wrong per comment #3)
(Assignee)

Comment 7

3 years ago
(In reply to Tomasz Kołodziejski [:tomasz] from comment #5)
> I'll be happy to dive into the search/suggestion code. I'm also happy that
> seeing ">>" at all is a bug as it looks really weird. So I guess this is not
> a feature but a regression, right?

Let me be clear (or at least I hope so, let me know if you have questions).

" >> " is used to implement what we call in-the-middle completion, that means it matches something in the value, but it's not at the beginning of it.
This is used for example in Thunderbird if you try to complete "Marco Bonardo - mbonardo@..." by typing "mbo", and you will get "mbo >> Marco Bonardo - mbonardo@..."

This is a feature by itself, not a regression and it's useful in Thunderbird.

Though, we did a clear choice in the Firefox urlbar that we NEVER want to complete in-the-middle cause the user is typing and we want to return only results that match at the beginning, for various reasons (we can match tags, title, url... we don't want to distract the user with unrelated results, want provide more value by restricting to specific behavior, and so on...).

Thus, the fact urlbar shows it in some edge-cases IS a regression.

There is no way (currently) to tell controller we don't want in-the-middle completion, so what we did is to only set defaultIndex if the found value and the string match at the beginning. this way we never enter the "else" branch that does in-the-middle completion.
Looks like this is an edge case where some condition causes us to hit that code path.

> I did a quick code search. Is the code responsible for that here:
> http://hg.mozilla.org/mozilla-central/file/e58842c764dd/toolkit/components/
> autocomplete/nsAutoCompleteController.cpp#l1637?

right.

> I'd welcome any more information about this bug. Please keep in mind that
> I'll see this code for the first time so I don't really know its mechanics.

I see 2 possibilities to fix this:
1. find the race condition that causes us to return something that doesn't match at the beginning
2. provide an attribute that allows a consumer to opt-out of in-the-middle completion.

I think even doing both might be a win in the end.
Flags: needinfo?(mak77)
(Assignee)

Comment 8

3 years ago
Marco, since my other bug is waiting for review, I'm taking this. please update the backlog accordingly.
Assignee: nobody → mak77
Status: NEW → ASSIGNED
Flags: in-testsuite?
OS: Mac OS X → All
Hardware: x86 → All
Added to IT 35.1

(In reply to Marco Bonardo [:mak] (needinfo? me) from comment #8)
> Marco, since my other bug is waiting for review, I'm taking this. please
> update the backlog accordingly.
Iteration: --- → 35.1
(Assignee)

Comment 10

3 years ago
Created attachment 8486651 [details] [diff] [review]
patch v1

This fixes the problem.

I had to remove an assertion (that I added in the past!) cause I was wrong.  The final value can also not match the input value, this is something completely up to the search provider to decide, the controller shouldn't have veto power over it.
It's the value that in our case should contain the input, but we can't assert that due to existence of in-the-middle completion.
Attachment #8486651 - Flags: review?(ttaubert)
Comment on attachment 8486651 [details] [diff] [review]
patch v1

Review of attachment 8486651 [details] [diff] [review]:
-----------------------------------------------------------------

LGTM

::: toolkit/components/places/UnifiedComplete.js
@@ +729,5 @@
>  
>      let match = yield PlacesSearchAutocompleteProvider.findMatchByToken(
>                                                             this._searchString);
>      if (match) {
> +      // The match doesn't contains a 'scheme://www.' prefix, but since we have

Nit: doesn't contain
Attachment #8486651 - Flags: review?(ttaubert) → review+
(Assignee)

Comment 12

3 years ago
with the nit fixed https://hg.mozilla.org/integration/fx-team/rev/b0e5c955b607
Flags: in-testsuite? → in-testsuite+
Target Milestone: --- → Firefox 35
https://hg.mozilla.org/mozilla-central/rev/b0e5c955b607
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED

Updated

3 years ago
QA Contact: alexandra.lucinet
QA Contact: alexandra.lucinet → andrei.vaida
Reproduced the bug with Nightly 35 (2014-08-28 / 20140828030205).

This is now verified fixed on Nightly 35 (2014-09-14), using Windows 7 64-bit, Ubuntu 14.04 LTS 64-bit and Mac OS X 10.9.4. Testing was based on Comment 0 and Comment 7.

Acceptance criteria for this patch:
- The ">>" symbol is no longer displayed for suggested URLs, as in-the-middle auto-completion is no longer occurring in the awesomebar.
Status: RESOLVED → VERIFIED
status-firefox35: --- → verified
Depends on: 1067602
You need to log in before you can comment on or make changes to this bug.