1478003 - Awesomebar visit action prefers http sites even when https:// is entered and in history

Status: VERIFIED FIXED

(Firefox :: Address Bar, defect, P1)

Description

[Gian-Carlo Pascutto [:gcp]]

Attachment: naughtyfox.png

In the past I have visited (a lot) http://zero.sjeng.org

The site was HTTPS enabled today. I have maybe a few visits to https://zero.sjeng.org

STR:

Click on Firefox Awesomebar. Enter https://zero.sjeng.org. Press Enter.

Actual result:

Firefox goes to http://zero.sjeng.org (insecure, and not what I typed)

Expected result:

Firefox goes to the site I entered.

The screenshot shows this in action, it seems to replace the Awesomebar entry by the top history hit.

Comment 1

[Marco Bonardo [:mak] (Away Apr 25 - May 5)]

This is exactly the opposite of what we want to happen, we should never-ever change what the user typed. Looks like some check has been wrongly removed here.

Comment 2

[Gian-Carlo Pascutto [:gcp]]

I found out something: it works correctly if I enter everything, including the training /. (This is what the autocomplete in blue suggests).

If I enter anything short of the entire suggested URL, including trailing slash, it fails.

Comment 3

[Marco Bonardo [:mak] (Away Apr 25 - May 5)]

This is what I see so far, we have both https://zero.sjeng.org and http://zero.sjeng.org in the origins table, the nonsecure version has an higher frecency.
Our query has a :prefix param, though, after the grouped search, we pick the prefix of the most frecent origin, that is http.
We should instead use :prefix if it's present.

This is a regression of the new autofill code.

Comment 4

[Marco Bonardo [:mak] (Away Apr 25 - May 5)]

Attachment: Bug 1478003 - Autofill may prefer the most frecent protocol even if a different one is provided. r=standard8

If the user types a protocol, autofill should respect it. Currently we pick the most frecent protocol instead.

Comment 5

[Phabricator Automation]

Comment on attachment 8994779 [details]
Bug 1478003 - Autofill may prefer the most frecent protocol even if a different one is provided. r=standard8

Mark Banner (:standard8) has approved the revision.

https://phabricator.services.mozilla.com/D2352

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/2607629d20b3
Autofill may prefer the most frecent protocol even if a different one is provided. r=Standard8

Comment 7

[Cristian Brindusan [:cbrindusan]]

https://hg.mozilla.org/mozilla-central/rev/2607629d20b3

Comment 8

[Marco Bonardo [:mak] (Away Apr 25 - May 5)]

Comment on attachment 8994779 [details]
Bug 1478003 - Autofill may prefer the most frecent protocol even if a different one is provided. r=standard8

Approval Request Comment
[Feature/Bug causing the regression]: bug 1239708
[User impact if declined]: We may override a user-typed protocol with a different one, in the worst case overriding a secure visit with an unsecure one
[Is this code covered by automated tests?]: yes
[Has the fix been verified in Nightly?]: not yet
[Needs manual test from QE? If yes, steps to reproduce]: yes, the required steps are:
1. visit http://mozilla.org once
2. visit http://zero.sjeng.org 3 times in separate tabs (don't reload)
3. visit https://zero.sjeng.org 2 times in separate tabs (don't reload)
4. now typing in the address bar "https://zero.sjeng.org" should suggest it as the first entry, it should not suggest "zero.sjeng.org" (without https)
[List of other uplifts needed for the feature/fix]: none
[Is the change risky?]: no
[Why is the change risky/not risky?]: it's a very simple oneline change to a query, this code has automated tests
[String changes made/needed]: none

Comment 9

[Cristian Fogel [:cfogel]]

Managed to reproduce on 63.0a1 (2018-07-21) on Ubuntu 16.04LTS and 62.0b10 on win10x64. Oddly enough on macOS I was unable to reproduce it.

Can confirm the fix is working on the current nightly: 63.0a1 (2018-07-26).

Comment 10

[Liz Henry (:lizzard) (relman/hg->git project)]

Comment on attachment 8994779 [details]
Bug 1478003 - Autofill may prefer the most frecent protocol even if a different one is provided. r=standard8

Fix for regression in 62, verified in nightly, let's uplift for beta 13.

Comment 11

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/releases/mozilla-beta/rev/f15db10c801c

Comment 12

[Cristian Fogel [:cfogel]]

Confirming fix on 62.0b13 as well.