new visits are inserted incorrectly in the Library and the sidebar treeviews

RESOLVED FIXED in Firefox 22

Status

()

Toolkit
Places
RESOLVED FIXED
5 years ago
4 years ago

People

(Reporter: Alan York, Assigned: mak)

Tracking

(Depends on: 1 bug, {regression})

21 Branch
mozilla24
x86
All
regression
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(firefox21 wontfix, firefox22+ verified, firefox23+ fixed, firefox24+ fixed, firefox-esr17 unaffected)

Details

Attachments

(1 attachment)

(Reporter)

Description

5 years ago
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.
(Reporter)

Updated

5 years ago
Component: Untriaged → Bookmarks & History

Comment 1

5 years ago
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
Blocks: 831094
Status: UNCONFIRMED → NEW
status-firefox21: --- → affected
status-firefox22: --- → affected
status-firefox23: --- → affected
status-firefox24: --- → affected
status-firefox-esr17: --- → unaffected
tracking-firefox22: --- → ?
tracking-firefox23: --- → ?
tracking-firefox24: --- → ?
Component: Bookmarks & History → Places
Ever confirmed: true
Keywords: regression
Product: Firefox → Toolkit

Updated

5 years ago
OS: Windows XP → All

Updated

5 years ago
Summary: new history displays incorrectly in existing window/sidebar → new history displays incorrectly in Library window/sidebar

Comment 2

5 years ago
:mak,
can you consider backing Bug831094 out from Firefox22beta/Aurora23.0a2?
Flags: needinfo?(mak77)
(Assignee)

Comment 3

5 years ago
(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?
Flags: needinfo?(mak77)
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.
Assignee: nobody → mak77
tracking-firefox22: ? → +
tracking-firefox23: ? → +
tracking-firefox24: ? → +
(Assignee)

Comment 5

5 years ago
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.
(Assignee)

Comment 6

5 years ago
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.
(Assignee)

Updated

5 years ago
Summary: new history displays incorrectly in Library window/sidebar → new visits are inserted incorrectly in the Library and the sidebar treeviews
(Assignee)

Comment 7

5 years ago
Created attachment 756936 [details] [diff] [review]
patch v1.0

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)
(Assignee)

Comment 8

5 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/8c37f4a3e488
Flags: in-testsuite+
Target Milestone: --- → mozilla24
(Assignee)

Comment 10

5 years ago
linux only failure...
(Assignee)

Comment 11

5 years ago
and the failure happens on the sanity check, doesn't make much sense :(
(Assignee)

Comment 12

5 years ago
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.
https://hg.mozilla.org/mozilla-central/rev/d8cfe163b77d
Status: NEW → RESOLVED
Last Resolved: 5 years ago
status-firefox24: affected → fixed
Resolution: --- → FIXED
(Assignee)

Comment 15

5 years ago
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
Attachment #756936 - Flags: approval-mozilla-beta?
Attachment #756936 - Flags: approval-mozilla-aurora?
Comment on attachment 756936 [details] [diff] [review]
patch v1.0

Low risk, fixes regression, tested - approving for uplift.
Attachment #756936 - Flags: approval-mozilla-beta?
Attachment #756936 - Flags: approval-mozilla-beta+
Attachment #756936 - Flags: approval-mozilla-aurora?
Attachment #756936 - Flags: approval-mozilla-aurora+
https://hg.mozilla.org/releases/mozilla-aurora/rev/3d63ffc038cf
https://hg.mozilla.org/releases/mozilla-beta/rev/13d9cc0d5428
status-firefox21: affected → wontfix
status-firefox22: affected → fixed
status-firefox23: affected → fixed
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.
status-firefox22: fixed → verified
QA Contact: manuela.muntean

Updated

5 years ago
Depends on: 892485

Comment 19

4 years ago
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.

Comment 20

4 years ago
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)
(Assignee)

Comment 22

4 years ago
(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.

Comment 23

4 years ago
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.