1687767 - Avoid highlight cost on unknown urls heuristic results

Status: VERIFIED FIXED

(Firefox :: Address Bar, task, P3)

Description

[Marco Bonardo [:mak]]

We're spending time on a couple things that are not particularly useful:

1. unknown urls, we just copy them over to the heuristic result without adding anything useful, and we fully highlight them when we do that.
2. long (url) strings, we still try to autofill them, that is expensive and doesn't bring anything useful to the user, the autofilled part, even if it existed, would be out of screen.

Comment 1

[Marco Bonardo [:mak]]

Attachment: Bug 1687767 - Don't highlight url heuristic results without autocompleted parts. r=harry

Avoid the cost of highligthing unvisited heuristic url results, since the whole
string would just be copied over and highlighted, paying an unnecessary cost.
Note, here we could go with an HIGHLIGHT.ALL mode, to retain the current behavior,
but highlighting the full url just makes it less readable, and we're not adding
anything to it, so highligting the typed part is not useful regardless.

Also don't try to autofill very long strings, it's expensive and not useful,
since the autofilled part would be out of screen anyway.

Comment 2

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/207bf1b29704
Don't highlight url heuristic results without autocompleted parts. r=harry

Comment 3

[Bogdan Tara[:bogdan_tara | bogdant]]

https://hg.mozilla.org/mozilla-central/rev/207bf1b29704

Comment 4

[Florencia Di Ciocco, NI to vbalducci]

Hi Marco,
Could you give me some str this? I'm a bit lost on how to verify that this bug has been fixed.

Thanks in advance, Flor.

Comment 5

[Marco Bonardo [:mak]]

basically, if you paste a url in the urlbar, the heuristic preselected visit result should not get a bold highlight, for example if you paste http://www.mozilla.org/
This is to save the costs of highligting very long urls.

Comment 6

[Florencia Di Ciocco, NI to vbalducci]

Thanks Marco,

Ok, I see what you mean. If you follow these steps, you still see the bold highlight.

Open FF
Open a new tab
Paste the URL (you see that the heuristic preselected visit result is not in bold highlight)
Open a new tab
Go back to the previous tab
Click on the urlbar again (the pasted URL is still there)
The heuristic preselected visit result gets a bold highlight

Comment 7

[Marco Bonardo [:mak]]

By chance when you go back to the first tab, is any part of the url being autofilled/selected in the input field?
If any kind of autofill happens, then we highlight normally. There may actually be a an edge case bug in autofill, where if I paste https://www.google.com/, and it's a known url, the last slash gets selected when I go back to the first tab.

The key is the "unknown" part in the title of the bug.
Basically this patch only acts when Firefox knows nothing about the pasted url, if the url is known to history or bookmarks, it may be autofilled and this patch doesn't act, because we still want to highlight a match.
If you try with https://www.google.com/search?q=randomstring, does it happen?

Comment 8

[Florencia Di Ciocco, NI to vbalducci]

Attachment: bookmarks?

I see, yes, I think that it got highlighted because of the bookmarks and/or landing page of a new profile. But if so, if it's being highlighted because of the bookmarks, why isn't bold highlighted the first time? Why does it need to lose focus to be bold highlighted?

On the other hand, with the random search string, the behavior is as expected.

Comment 9

[Marco Bonardo [:mak]]

(In reply to Florencia Di Ciocco from comment #8)

I see, yes, I think that it got highlighted because of the bookmarks and/or landing page of a new profile. But if so, if it's being highlighted because of the bookmarks, why isn't bold highlighted the first time? Why does it need to lose focus to be bold highlighted?

It's complex. On paste we disallow autofill, so the first time it won't activate. But when you switch to another tab and back, we allow autofill. Then autofill activates and adds and selects /. I think that's a behavior due to a combination of things. If I read it correctly, on blur we actually format the url and remove / (indeed if you blur the urlbar switch to another tab and back, the / is gone), then on autofill we add back the slash. It could be solved by bug 1577539, because then we'd not format the string.

Comment 10

[Florencia Di Ciocco, NI to vbalducci]

I understand. Thanks so much for taking the time of explaining this. I've uploaded the bug status and the flags.