Closed
Bug 496542
Opened 16 years ago
Closed 16 years ago
Duplicate entries in History sidebar and Library
Categories
(Firefox :: Bookmarks & History, defect)
Tracking
()
VERIFIED
FIXED
Firefox 3.6a1
People
(Reporter: john.p.baker, Assigned: mak)
References
Details
(Keywords: regression, verified1.9.1)
Attachments
(1 file, 1 obsolete file)
(deleted),
patch
|
Details | Diff | Splinter Review |
User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9.1pre) Gecko/20090604 Shiretoko/3.5pre
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9.1pre) Gecko/20090604 Shiretoko/3.5pre
If you use 'Most Visited' to go to a history entry the sidebar shows more than one entry for that page (probably one for each visit made previously)
Reproducible: Always
Steps to Reproduce:
1. View Sidebar / History, set 'By Last Visited', close sidebar
2. Go to http://planet.mozilla.org/ from location bar
3. Go to http://www.bristol.ac.uk/ from location bar
4. repeat 2 and 3 a few times possibly with an occasional extra site
5. From "Most Visited" select "Planet Mozilla"
6. View Sidebar / History
Actual Results:
"Planet Mozilla" appears more than once at the top of the list
Expected Results:
"Planet Mozilla" to appear once
Reporter | ||
Updated•16 years ago
|
Keywords: regression
Version: unspecified → 3.5 Branch
Assignee | ||
Comment 1•16 years ago
|
||
confirmed, really originally i was recalling by last visited was not grouping pages, but it is. sounds like something is broken in updating the result in one of the 2 cases.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Reporter | ||
Comment 2•16 years ago
|
||
If you leave a couple of minutes between steps 5 and 6 (or AFAICT viewing the history sidebar again after more than a couple of minutes have passed) the results are correct - which might imply that the temp tables are involved.
Reporter | ||
Comment 3•16 years ago
|
||
I have just encountered a slightly different version of this after restarting the browser to a saved session; I am having problems reducing this to STR but it looks as if a non-cache page in a tab in a saved session will show multiple entries at that entries location in the sidebar (ie the bug is not only for the first entry)
Comment 4•16 years ago
|
||
Not only in Sidebar but in "Show All History" view too.
So, on "Today" list you can see page occurrence multiplied by ~5, on Last 7 day multplied ~10 .. and so on.
Sometimes bug happens, sometimes not.
Comment 6•16 years ago
|
||
Note that this bug can be triggered by clicking the home button multiple times too, as in bug 498335. The method of navigating to pages (i.e., using Most Visited as indicated in comment 0 or not) most likely doesn't matter.
Keywords: regressionwindow-wanted
OS: Windows 2000 → All
Hardware: x86 → All
Summary: Multiple entries on History sidebar (by Last Visited) → Duplicate entries in History sidebar and Library
Updated•16 years ago
|
Flags: wanted-firefox3.6+
Comment 7•16 years ago
|
||
Regression range:
Woks Fine:
http://hg.mozilla.org/mozilla-central/rev/a1fc27041e53
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2a1pre) Gecko/20090514 Minefield/3.6a1pre ID:20090514043824
Broken:
http://hg.mozilla.org/mozilla-central/rev/b766e9dc2335
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2a1pre) Gecko/20090515 Minefield/3.6a1pre ID:20090515044249
Pushlog:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=a1fc27041e53&tochange=b766e9dc2335
Comment 8•16 years ago
|
||
Regression range in 1.9.1:
Works fine:
http://hg.mozilla.org/releases/mozilla-1.9.1/rev/e49c05fc9122
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1b5pre) Gecko/20090519 Shiretoko/3.5b5pre ID:20090519043912
Broken:
http://hg.mozilla.org/releases/mozilla-1.9.1/rev/acd2d4638228
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1b5pre) Gecko/20090520 Shiretoko/3.5b5pre ID:20090520041838
Pushlog:
http://hg.mozilla.org/releases/mozilla-1.9.1/pushloghtml?fromchange=e49c05fc9122&tochange=acd2d4638228
Reporter | ||
Comment 9•16 years ago
|
||
This could implicate bug 488966; One odd thing about the patch in that bug is that the following phrase in the old code
- "SELECT id, url, title, rev_host, visit_count, visit_date, "
- "favicon_url, session, empty "
- "FROM ( "
- "SELECT h.id AS id, h.url AS url, h.title AS title, "
- "h.rev_host AS rev_host, h.visit_count AS visit_count, "
- "MAX(v.visit_date) AS visit_date, f.url AS favicon_url, "
- "v.session AS session, null AS empty "
- "FROM moz_places_temp h "
- "JOIN moz_historyvisits v ON h.id = v.place_id "
- "LEFT JOIN moz_favicons f ON h.favicon_id = f.id "
- "WHERE h.hidden <> 1 AND v.visit_type NOT IN ") +
- nsPrintfCString("(0,%d) ",
- nsINavHistoryService::TRANSITION_EMBED) +
- NS_LITERAL_CSTRING("{ADDITIONAL_CONDITIONS} "
- "AND h.id NOT IN (SELECT id FROM moz_places_temp) "
- "GROUP BY h.id "
- ") "
is, AFAICT, a no-op as by definition h.id is an 'id FROM moz_places_temp'. This is no longer the case in the new code.
+ "SELECT h.id, h.url, h.title, h.rev_host, h.visit_count, "
+ "h.last_visit_date, f.url, v.session, null "
+ "FROM moz_places_temp h "
+ "JOIN moz_historyvisits v ON h.id = v.place_id "
+ "LEFT JOIN moz_favicons f ON h.favicon_id = f.id "
+ // WHERE 1 is a no-op since additonal conditions will start with AND.
+ "WHERE 1 "
+ "{QUERY_OPTIONS_VISITS} {QUERY_OPTIONS_PLACES} "
+ "{ADDITIONAL_CONDITIONS} "
Keywords: regressionwindow-wanted
Assignee | ||
Comment 10•16 years ago
|
||
yes, it is for sure a problem with table partitioning and the query. And i second you on the fact the old query was executing a no-op.
Blocks: 488966
Assignee | ||
Comment 11•16 years ago
|
||
ugh, the GROUP BY is not applied to global UNIONed result, but only to a single query :\ this applies to all selectAsURI queries.
Since actually those queries are the most used all around, if possible this should be fixed for next RC if it will exists.
I'm asking blocking status to make drivers aware, it's not an hard blocker, but all of our main history queries are based upon this query, users are most likely to notice a bunch of entries for the same page till temp tables are synced to disk tables. So on every new added visit for 2 minutes.
Also, this needs a unit test, of course.
Assignee: nobody → mak77
Flags: wanted-firefox3.5?
Flags: in-testsuite?
Flags: blocking-firefox3.5?
Comment 12•16 years ago
|
||
This also affects "Forget this Site"; if you right-click on a website and ask Firefox to forget it, the experience is a little concerning. It in face removes the entry you're looking at, then that entry comes back for a few seconds, then it goes away again.
This might require an RC2.
Assignee | ||
Comment 13•16 years ago
|
||
This should fix all of the above, works fine from my tests, but everyone able to see the issue and to compile could try this for added safety.
Attachment #383476 -
Flags: review?(sdwilsh)
Assignee | ||
Updated•16 years ago
|
Whiteboard: [needs review sdwilsh]
Assignee | ||
Comment 14•16 years ago
|
||
PS: from a perf point of view this will lose something, but on a large db where the actual query takes 2s (notice old query was taking almost 4s), this will lose about 50-100ms, not a so big concern.
No reasons to cry on a perf loss if the query is wrong, though.
It's easy to go fast if you don't care about the results!
Can we get a try server build going ASAP for wider testing?
Flags: wanted1.9.1.x+
Assignee | ||
Comment 16•16 years ago
|
||
try server building changeset ee807dcde041
Comment 17•16 years ago
|
||
Comment on attachment 383476 [details] [diff] [review]
patch v1.0
>+++ b/toolkit/components/places/tests/sync/test_database_sync_with_specialHistoryQueries.js
>+ // Set the preference for the timer to a large value so we don't sync.
>+ prefs.setIntPref(kSyncPrefName, 1);
This is not a large value
r=sdwilsh
Attachment #383476 -
Flags: review?(sdwilsh) → review+
Updated•16 years ago
|
Whiteboard: [needs review sdwilsh]
Comment 19•16 years ago
|
||
Status: NEW → RESOLVED
Closed: 16 years ago
Resolution: --- → FIXED
Comment 20•16 years ago
|
||
per shaver's email to release-drivers, marking this blocking.
Flags: wanted1.9.1.x+
Flags: wanted-firefox3.6+
Flags: wanted-firefox3.5?
Flags: blocking-firefox3.5?
Flags: blocking-firefox3.5+
Comment 21•16 years ago
|
||
Verified fixed on trunk with builds on all platforms like:
Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.2a1pre)
Gecko/20090616 Minefield/3.6a1pre ID:20090616111535
Status: RESOLVED → VERIFIED
Target Milestone: --- → Firefox 3.6a1
Comment 22•16 years ago
|
||
Status: VERIFIED → RESOLVED
Closed: 16 years ago → 16 years ago
Target Milestone: Firefox 3.6a1 → ---
Updated•16 years ago
|
Keywords: fixed1.9.1
Comment 23•16 years ago
|
||
restoring v. and tm...
A test landed with this as well, so in-testsuite+
Status: RESOLVED → VERIFIED
Flags: in-testsuite? → in-testsuite+
Target Milestone: --- → Firefox 3.6a1
Comment 24•16 years ago
|
||
Verified fixed on 1.9.1 with builds on all platforms like:
Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1) Gecko/20090616 Firefox/3.5 ID:20090616212246
One remaining question to "Forget about this site". When I run this action for a sub domain only this subdomain will be removed. But running it on the domain itself, everything should be removed with the domain name in it. Is that correct?
Keywords: fixed1.9.1 → verified1.9.1
You need to log in
before you can comment on or make changes to this bug.
Description
•