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

Status: RESOLVED FIXED

(Firefox :: Address Bar, defect)

Description

[Loic]

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.

Comment 1

[Alice0775 White]

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

Comment 2

[Marco Bonardo [:mak]]

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?

Comment 3

[Loic]

Attachment: 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.

Comment 4

[Marco Bonardo [:mak]]

ok, I think I figured this out as a bug in the delete trigger, doesn't check for 'www'. thanks.

Comment 5

[Marco Bonardo [:mak]]

Attachment: 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.

Comment 6

[Dietrich Ayala (: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?

Comment 7

[Marco Bonardo [:mak]]

(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 8

[Dietrich Ayala (:dietrich)]

Comment on attachment 597892 [details] [diff] [review]
patch v1.0

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

ok, thanks!

Comment 9

[Marco Bonardo [:mak]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/75bb9debefe9

Comment 10

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/75bb9debefe9