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

VERIFIED FIXED in Firefox 62

Status

()

defect
P1
normal
VERIFIED FIXED
11 months ago
11 months ago

People

(Reporter: gcp, Assigned: mak)

Tracking

({regression})

unspecified
Firefox 63
x86_64
Linux
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox-esr52 unaffected, firefox-esr60 unaffected, firefox61 unaffected, firefox62+ verified, firefox63+ verified)

Details

(Whiteboard: [fxsearch])

Attachments

(2 attachments)

Reporter

Description

11 months ago
Posted image 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.
Assignee

Comment 1

11 months ago
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.
Priority: -- → P1
Whiteboard: [fxsearch]
Assignee

Updated

11 months ago
Assignee: nobody → mak77
Status: NEW → ASSIGNED
Reporter

Comment 2

11 months ago
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.
Assignee

Comment 3

11 months ago
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.
Assignee

Comment 4

11 months ago
If the user types a protocol, autofill should respect it. Currently we pick the most frecent protocol instead.
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
Attachment #8994779 - Flags: review+

Updated

11 months ago
Attachment #8994779 - Attachment description: Bug 1478003 - Autofill may prefer the most frecenct protocol even if one is provided. r=standard8 → Bug 1478003 - Autofill may prefer the most frecent protocol even if one is provided. r=standard8

Updated

11 months ago
Attachment #8994779 - Attachment description: Bug 1478003 - Autofill may prefer the most frecent protocol even if one is provided. r=standard8 → Bug 1478003 - Autofill may prefer the most frecent protocol even if a different one is provided. r=standard8

Comment 6

11 months ago
Pushed by mak77@bonardo.net:
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

11 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/2607629d20b3
Status: ASSIGNED → RESOLVED
Closed: 11 months ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 63
Assignee

Comment 8

11 months ago
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
Attachment #8994779 - Flags: approval-mozilla-beta?
Flags: qe-verify+
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).
Status: RESOLVED → VERIFIED
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.
Attachment #8994779 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Confirming fix on 62.0b13 as well.
Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.