1045283 - Unified Autocomplete inline auto-fill results don't have a favicon when redirects/HSTS are involved

Status: NEW

(Firefox :: Address Bar, defect, P5)

Description

[Matthew N. [:MattN]]

Attachment: Screenshot of Unified Autocomplete behaviour for "f"

STR:
1) Type "f" in the awesomebar

Expected (Before Unified Autocomplete): 
* inline autofill of "facebook.com/"
* First entry in the dropdown is "https://www.facebook.com/" with a nice favicon
* Second entry in the dropdown is "http://cloud.feedly.com/"

Actual (with Unified Autocomplete):
* inline autofill of "facebook.com/" (same as expected)
* First entry in the dropdown is "facebook.com/" ("http://facebook.com/" upon highlight) with no favicon
* Second entry in the dropdown is "https://www.facebook.com/" (effectively the same as the first result)

The issue is the lack of favicon due to STS/redirects and that the two results are effectively the same (adding a keystroke to get to Feedly).

Facebook has had the header "Strict-Transport-Security: max-age=7776000" for months IIRC so we should have enough data to know that the first two results are effectively the same.

Comment 1

[Matthew N. [:MattN]]

Attachment: Screenshot of Unified Autocomplete behaviour for "y"

A similar case applies for me with Yammer. I bookmarked the "All company" view since that's what I always want to see but now "y" gives me "http://yammer.com/" with no favicon instead of "https://www.yammer.com/mozilla.com/#/threads/company?type=general" with a favicon.

Comment 2

[Marco Bonardo [:mak]]

we don't have "best favicon for a domain" yet, an easy solution might be to introduce an "autoFill" icon, or just style the autoFill entry differently.

Comment 3

[Marco Bonardo [:mak]]

the fact we show both facebook.com and www.facebook.com is unfortunate but iirc there's a technical constraint by which we can't coalesce them ignoring www. Unfortunately I was so dumb to not annotate the reason in a comment, but by doing that I'm sure one of the tests fails and remembers us the reason.

Comment 4

[Matthew N. [:MattN]]

(In reply to Marco Bonardo [:mak] from comment #2)
> we don't have "best favicon for a domain" yet

I don't think such a thing would be a good idea anyways since there can be multiple icons for a single domain. e.g. google.com/ and www.google.com/analytics/web/. Since we already have the data in places for each place, this shouldn't be necessary.

> an easy solution might be to
> introduce an "autoFill" icon, or just style the autoFill entry differently.

Perhaps if it wasn't a full-height row (e.g. just the domain) and we had the bug implemented to select the row by default then that could work.

Have we considered using nsISiteSecurityService.isSecureHost to ensure we don't ever show http results for a site using HSTS?

(In reply to Marco Bonardo [:mak] from comment #3)
> the fact we show both facebook.com and www.facebook.com is unfortunate but
> iirc there's a technical constraint by which we can't coalesce them ignoring
> www.

Knowing the reason would be very useful to understand how hard that problem would be to fix.

Comment 5

[Marco Bonardo [:mak]]

(In reply to Matthew N. [:MattN] from comment #4)
> (In reply to Marco Bonardo [:mak] from comment #2)
> > we don't have "best favicon for a domain" yet
> 
> I don't think such a thing would be a good idea anyways since there can be
> multiple icons for a single domain. e.g. google.com/ and
> www.google.com/analytics/web/. Since we already have the data in places for
> each place, this shouldn't be necessary.

We could likely limit to known favicons for the root domain (if we fetch the root domain favicon, we can store a link to it for the host). Or try to fetch it in background (we never did this for privacy reasons though). Favicons for paths would be ignored.

> > an easy solution might be to
> > introduce an "autoFill" icon, or just style the autoFill entry differently.
> 
> Perhaps if it wasn't a full-height row (e.g. just the domain) and we had the
> bug implemented to select the row by default then that could work.

select by default is the plan (bug 1045105). Regarding the full-height-row, I fear it would break the popup contents rhythm, and we spent quite a lot of time in the past trying to get that right. UX directions might be good for this.

> Have we considered using nsISiteSecurityService.isSecureHost to ensure we
> don't ever show http results for a site using HSTS?

What's the point if it's using hsts, it will end up there regardless, we would only be saving a transparent server side redirect (it doesn't even cause a reload). Fwiw we are already forcing https if the user types the https url. What's the relation with this bug?

> (In reply to Marco Bonardo [:mak] from comment #3)
> > the fact we show both facebook.com and www.facebook.com is unfortunate but
> > iirc there's a technical constraint by which we can't coalesce them ignoring
> > www.
> 
> Knowing the reason would be very useful to understand how hard that problem
> would be to fix.

Having a bug filed will be a good starting point, I currently found that a unified complete test has remained disabled without a reasons, if I have the time while looking at that I'll try to rememeber the reasoning.

Comment 6

[Marco Bonardo [:mak]]

(In reply to Marco Bonardo [:mak] from comment #5)
> What's the relation with this bug?

Ehr sorry I made confusion with other bugs and ended up replying wrongly.

So, there are 2 bugs:

1. there is no icon for autoFilled entries regardless of them being htst or whatever cause we can't guess the best icon for a domain we built up. And as you said it's hard to guess correctly, even if something could be done (as I said).

2. in some cases we might show multiple entries (secure, non secure, www). We could try to coalesce harder with post-filtering, but what happens if your page serves 2 different contents based on http or https?
Using hsts and redirects is a good guess, but I think it will be hard to use it in an efficient way (might require complex code, expensive to maintain). We should also figure a way to stop doing that if something changes server side (what if hsts or redirect is removed suddenly?). Sure won't be an issue for facebook but might be for millions other pages.
It's also complicated by the fact the inline result you see ("facebook.com") might already be going directly to the secure version, that's mostly a cosmetic bug (we want to show the same as you are seing in the input field, even if it's not the final destination).

I think I might look into improving the filtering by using the final autoFill value instead of the shown value though, that could make sense.

Comment 7

[Marco Bonardo [:mak]]

the problem of keeping both www. and non-www version is due to the fact we'd end up retaining the first entry we get and discarding the second one.
As it is now searching "test" you will likely get both www.test/ and test/. We don't know which result will come first and it would be worse if we'd not suggest test/ on a "www.test" search just cause the user never visited the www version until that time. Also the awesomebar has always ignored www prefix so this copes with its behavior.
The only way to filter this more properly would be to buffer results, that means every search would be much slower though.
We try to give as more useful results as possible to the user, that means in some cases we might be giving more than needed to avoid giving less.

Comment 8

[Marco Bonardo [:mak]]

I will try to make a test patch that might improve filtering situation in another bug.

Comment 9

[Marco Bonardo [:mak]]

This is not actionable as we need a fix for bug 1664001 first, and likely that will also solve this (from the favicon point of view, as deduping is a totally different problem, but we can't track both in a single bug report).
So we'll keep bug 1664001 in the backlog and lower this to a P5.