Last Comment Bug 722176 - Inline autocomplete becomes inactive after deleting an entry in location bar history
: Inline autocomplete becomes inactive after deleting an entry in location bar ...
Status: RESOLVED FIXED
[inline-autocomplete:normal]
:
Product: Firefox
Classification: Client Software
Component: Location Bar (show other bugs)
: 12 Branch
: All All
: -- normal (vote)
: Firefox 13
Assigned To: Marco Bonardo [::mak]
:
Mentors:
Depends on: 734351
Blocks: 566489
  Show dependency treegraph
 
Reported: 2012-01-29 10:42 PST by Loic
Modified: 2012-03-26 10:41 PDT (History)
9 users (show)
mak77: in‑testsuite+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
snapshots_location_bar_bug722176 (94.17 KB, image/png)
2012-02-07 06:29 PST, Loic
no flags Details
patch v1.0 (6.77 KB, patch)
2012-02-16 10:46 PST, Marco Bonardo [::mak]
dietrich: review+
Details | Diff | Splinter Review

Description Loic 2012-01-29 10:42:45 PST
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 2012-01-29 13:12:20 PST
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] 2012-02-07 01:10:21 PST
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 2012-02-07 06:29:49 PST
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.
Comment 4 Marco Bonardo [::mak] 2012-02-11 05:15:28 PST
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] 2012-02-16 10:46:28 PST
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.
Comment 6 Dietrich Ayala (:dietrich) 2012-02-17 15:43:36 PST
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] 2012-02-17 16:15:27 PST
(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) 2012-02-21 12:29:18 PST
Comment on attachment 597892 [details] [diff] [review]
patch v1.0

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

ok, thanks!
Comment 10 Ed Morley [:emorley] 2012-02-22 16:02:33 PST
https://hg.mozilla.org/mozilla-central/rev/75bb9debefe9

Note You need to log in before you can comment on or make changes to this bug.