Closed Bug 874407 Opened 6 years ago Closed 6 years ago
new visits are inserted incorrectly in the Library and the sidebar treeviews
User Agent: Mozilla/5.0 (Windows NT 5.1; rv:21.0) Gecko/20100101 Firefox/21.0 (Beta/Release) Build ID: 20130511120803 Steps to reproduce: With either the history sidebar opened and set to display last visited, or the history window open and showing today's history, visited a website. Actual results: The most recent history entry was overwritten by the currently visited page - the correct history list only appears after closing and reopening the sidebar or history window. Expected results: The most recent history entry should be inserted at the top of the list with previous entries displayed beneath it.
I can reproduce the problem. STR: 1. Open history sidebar last visited view / Library history right pane URL1 URL2 URL3 2. Click to open URL3 Actual Results: URL3 URL2 URL3 URL1 was overwritten Expected results: URL3 URL1 URL2 Regression window(m-c) Good: http://hg.mozilla.org/mozilla-central/rev/325397090d12 Mozilla/5.0 (Windows NT 6.1; WOW64; rv:21.0) Gecko/20130206 Firefox/21.0 ID:20130206135631 Bad: http://hg.mozilla.org/mozilla-central/rev/4a6b8dd4dfe3 Mozilla/5.0 (Windows NT 6.1; WOW64; rv:21.0) Gecko/20130206 Firefox/21.0 ID:20130206180753 Pushlog: http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=325397090d12&tochange=4a6b8dd4dfe3 Regression window(m-i) Good: http://hg.mozilla.org/integration/mozilla-inbound/rev/03232247aef8 Mozilla/5.0 (Windows NT 6.1; WOW64; rv:21.0) Gecko/20130206 Firefox/21.0 ID:20130206113128 Bad: http://hg.mozilla.org/integration/mozilla-inbound/rev/2308bb67a6fa Mozilla/5.0 (Windows NT 6.1; WOW64; rv:21.0) Gecko/20130206 Firefox/21.0 ID:20130206122830 Pushlog: http://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=03232247aef8&tochange=2308bb67a6fa Regressed by: 6b183775ad02 Marco Bonardo — Bug 831094 - Avoid replacing nodes in Places query results. r=Mano sr=gavin
Status: UNCONFIRMED → NEW
Component: Bookmarks & History → Places
Ever confirmed: true
Product: Firefox → Toolkit
Summary: new history displays incorrectly in existing window/sidebar → new history displays incorrectly in Library window/sidebar
:mak, can you consider backing Bug831094 out from Firefox22beta/Aurora23.0a2?
(In reply to Alice0775 White from comment #2) > :mak, > can you consider backing Bug831094 out from Firefox22beta/Aurora23.0a2? no, that would break far more than what we may fix. We build on top of that and that is also a very convenient performance win. Why the question?
Marco if you can provide a low risk forward fix for this in the next week or two we should be able to uplift to Beta and ship without this regression in our next cycle.
The problem is in the view, I wrote a test for the backend and it passed properly, everything looks good debugging there. Instead the treeview gets the wrong index of the node to be moved from _getRowForNode. Still investigating the reason and will write a chrome test for it.
And that happens cause the backend moves the node in the result before notifying the frontend, the frontend then asks the backend for the index of the child to be moved, but it gets the new position, not the old one.
Summary: new history displays incorrectly in Library window/sidebar → new visits are inserted incorrectly in the Library and the sidebar treeviews
The fix is be safe enough for backporting. I modified an existing test but didn't rename it cause there are some bugs filed about random failures and I want to keep those valid for now.
Attachment #756936 - Flags: review?(mano)
Attachment #756936 - Flags: review?(mano) → review+
Target Milestone: --- → mozilla24
Backed out for failures in test_bug549192.xul: https://tbpl.mozilla.org/php/getParsedLog.php?id=23711996&tree=Mozilla-Inbound https://hg.mozilla.org/integration/mozilla-inbound/rev/6fc06b7fa44d
linux only failure...
and the failure happens on the sanity check, doesn't make much sense :(
Dumb mistake from my site, I should not have used post increment, pointlessy error-prone, as demonstrated. Due to that, 2 visits were being added with the same time, the natural ordering on linux differs and causes the failure. I'm just going to repush, but using pre increment in the test.
Comment on attachment 756936 [details] [diff] [review] patch v1.0 This fix is quite safe and comes with a test, I think it's worth an uplift. [Approval Request Comment] Bug caused by (feature/regressing bug #): bug 831094 User impact if declined: history treeviews (Library and sidebar) are wrongly updated and may end up in misbehaviors of commands on zombie rows. Plus the most recent visit data is not reported. Testing completed (on m-c, etc.): m-c Risk to taking this patch (and alternatives if risky): the patch is simple and contained, similar code is used in other known-to-be-good parts. Test included. String or IDL/UUID changes made by this patch: none
Comment on attachment 756936 [details] [diff] [review] patch v1.0 Low risk, fixes regression, tested - approving for uplift.
Verified fixed using the STR from comment 1, with Firefox 22 beta 4 (build ID: 20130605070403) on both Ubuntu 12.10 32bit and Mac OSX 10.8.3 in 32bit mode.
I've tried again with the current release Firefox 25.0 and Firefox 25.0b12 (Mozilla/5.0 (X11; Linux x86_64; rv:25.0) Gecko/20100101 Firefox/25.0) :I'm still seeing the same issue reported here https://bugzilla.mozilla.org/show_bug.cgi?id=900525,links in the history sidebar cannot be clicked on in sequence,as new history entries are added the highlighted item moves erratically up and down the list.
Since I'm still seeing this bug in the latest betas,do I have to file a new bug?
(In reply to msth67 from comment #19) > I've tried again with the current release Firefox 25.0 and Firefox 25.0b12 > (Mozilla/5.0 (X11; Linux x86_64; rv:25.0) Gecko/20100101 Firefox/25.0) :I'm > still seeing the same issue reported here > https://bugzilla.mozilla.org/show_bug.cgi?id=900525,links in the history > sidebar cannot be clicked on in sequence,as new history entries are added > the highlighted item moves erratically up and down the list. On Ubuntu 13.10 64-bit, I can see the same behavior with both Firefox 20 and Firefox 25: the highlighted link jumps erratically up and down the history list in the sidebar,and new links are added between the one just clicked and the next highlighted link (as mentioned in https://bugzilla.mozilla.org/show_bug.cgi?id=900525#c0)
(In reply to msth67 from comment #20) > Since I'm still seeing this bug in the latest betas,do I have to file a new > bug? I think the issue you are reporting is not this bug, this bug was replacing in-place a page with another one, causing a dataloss, while your issue is a page jumping in the list. I think there are other reports of this issue, but it's not this bug.
Yes,I think you're actually right:for some reason I assumed that the issue I've originally reported here https://bugzilla.mozilla.org/show_bug.cgi?id=900525 (and which,BTW,is still unsolved to date in every beta/nightly build I've tried so far) was somehow related to this one.
You need to log in before you can comment on or make changes to this bug.