Inline autocomplete becomes inactive after deleting an entry in location bar history

RESOLVED FIXED in Firefox 13

Status

()

Firefox
Location Bar
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: Loic, Assigned: mak)

Tracking

12 Branch
Firefox 13
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [inline-autocomplete:normal])

Attachments

(2 attachments)

(Reporter)

Description

6 years ago
User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:12.0a1) Gecko/20120129 Firefox/12.0a1
Build ID: 20120129031114

Steps to reproduce:

I browsed almost all the bug reports about inline autocomplete and I don't think this one has been filled, but I'm not completely aware of inline autocomplete behavior/bugs. So maybe it's invalid.

STR:

0) Start with a new fresh profile for Nightly
1) Open a new tab
2) Type "nytimes.com" (without quotes) and press Enter
3) Open a 2nd new tab and close the previous tab
4) Type "ny" (NB: inline autocomplete works)
5) Delete the 1st entry (www.nytimes.com with favicon) OR the 2nd entry (nytimes.com without favicon) in location bar history
6) Open a 2nd new tab and close the previous tab
7) Type "ny" (NB: inline autocomplete doesn't work)

NB: opening/closing tabs in steps 3 & 6 is not necessary, but it's more clear like that.

I observed that with various domains, deleting an entry in location bar history for this domain, disable inline autocomplete for this domain.
(Reporter)

Updated

6 years ago
Blocks: 566489
Component: Untriaged → Location Bar
OS: Windows 7 → All
Hardware: x86_64 → All

Comment 1

6 years ago
I can reproduce.
http://hg.mozilla.org/mozilla-central/rev/aee879a3190a
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:12.0a1) Gecko/20120129 Firefox/12.0a1 ID:20120129031114
Status: UNCONFIRMED → NEW
Ever confirmed: true
(Assignee)

Comment 2

6 years ago
Not sure if this is a bug, if you remove all the entries for a certain domain from history inline doesn't have anymore data to return.
So, after you remove the nytimes.com entry, do you still have any other nytimes.com entries in history?
(Assignee)

Updated

6 years ago
Whiteboard: [inline-autocomplete:needinfo]
(Reporter)

Comment 3

6 years ago
Created attachment 595010 [details]
snapshots_location_bar_bug722176

(In reply to Marco Bonardo [:mak] from comment #2)
> Not sure if this is a bug, if you remove all the entries for a certain
> domain from history inline doesn't have anymore data to return.
> So, after you remove the nytimes.com entry, do you still have any other
> nytimes.com entries in history?

In my STR in comment #0, I don't remove all the entries for nytimes.com domain, but just one entry in the location bar history.

See my snaphots of the location bar after steps 4, 5 and 7. In step 7, inline autocomplete becomes inactive.
(Assignee)

Comment 4

6 years ago
ok, I think I figured this out as a bug in the delete trigger, doesn't check for 'www'. thanks.
Assignee: nobody → mak77
Status: NEW → ASSIGNED
Whiteboard: [inline-autocomplete:needinfo] → [inline-autocomplete:normal]
(Assignee)

Comment 5

6 years ago
Created attachment 597892 [details] [diff] [review]
patch v1.0

As said the trigger was wrong. this includes a test for it.
I also replicated the same logic across other locations, since it's healthier.
Attachment #597892 - Flags: review?(dietrich)
Comment on attachment 597892 [details] [diff] [review]
patch v1.0

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

::: toolkit/components/places/Database.cpp
@@ +1647,5 @@
>      "INSERT OR IGNORE INTO moz_hosts (host, frecency) "
>          "SELECT fixup_url(get_unreversed_host(h.rev_host)) AS host, "
>                 "(SELECT MAX(frecency) FROM moz_places "
> +                "WHERE rev_host = get_unreversed_host(host || '.') || '.' "
> +                   "OR rev_host = get_unreversed_host(host || '.') || '.www.') "

not reversing host anymore? can you explain what's going on here?

also, should put the query bit into a shared string instead of duplicating around?
(Assignee)

Comment 7

6 years ago
(In reply to Dietrich Ayala (:dietrich) from comment #6)
> Comment on attachment 597892 [details] [diff] [review]
> patch v1.0
> 
> Review of attachment 597892 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: toolkit/components/places/Database.cpp
> @@ +1647,5 @@
> >      "INSERT OR IGNORE INTO moz_hosts (host, frecency) "
> >          "SELECT fixup_url(get_unreversed_host(h.rev_host)) AS host, "
> >                 "(SELECT MAX(frecency) FROM moz_places "
> > +                "WHERE rev_host = get_unreversed_host(host || '.') || '.' "
> > +                   "OR rev_host = get_unreversed_host(host || '.') || '.www.') "
> 
> not reversing host anymore? can you explain what's going on here?

I'm not sure I got the question right, though I'll try to explain.
rev_host may be something like moc.allizom. or moc.allizom.www.
host can just be mozilla.com, though with this info I have to match either of the 2 above.
So get_unreversed_host (host || '.') returns moc.allizom, and to that I either append '.' or '.www.' to obtain the string I want.

> also, should put the query bit into a shared string instead of duplicating
> around?

I prefer not, since I want to be able to read the queries without having to parse them mentally with pieces coming from everywhere :)
Comment on attachment 597892 [details] [diff] [review]
patch v1.0

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

ok, thanks!
Attachment #597892 - Flags: review?(dietrich) → review+
(Assignee)

Comment 9

6 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/75bb9debefe9
Target Milestone: --- → Firefox 13
(Assignee)

Updated

6 years ago
Flags: in-testsuite+
https://hg.mozilla.org/mozilla-central/rev/75bb9debefe9
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
(Assignee)

Updated

6 years ago
Depends on: 734351

Updated

6 years ago
QA Contact: untriaged → location.bar
You need to log in before you can comment on or make changes to this bug.