Deleting history items doesn't remove them from ui immediately

VERIFIED FIXED in mozilla0.9.2

Status

()

Core
History: Global
P2
normal
VERIFIED FIXED
17 years ago
17 years ago

People

(Reporter: Alec Flett, Assigned: Alec Flett)

Tracking

Trunk
mozilla0.9.2
x86
Windows 2000
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: waiting for 11412 before i can verify)

Attachments

(2 attachments)

(Assignee)

Description

17 years ago
When you delete an item, or even an entire hostname or domain from history, you
don't get immediate feedback - instead you have to collapse and re-expand the
twisty.. this is bad because it makes it look like delete doesn't work at all!

The problem is that the RDF notifications aren't firing for find: URIs - we're
correctly unasserting from the history root, but not constructing "likely" find:
uris.

This MIGHT be a dupe, but I can't find it...
(Assignee)

Comment 1

17 years ago
nominating for nsbeta1, 0.9.3, which I think means RTM
Keywords: nsbeta1
Priority: -- → P2
Target Milestone: --- → mozilla0.9.3

Comment 2

17 years ago
nav triage team:

Gotta fix this one if we fix anything more for history. Marking nsbeta1+ and 
mozilla0.9.2
Keywords: nsbeta1 → nsbeta1+
Target Milestone: mozilla0.9.3 → mozilla0.9.2

Updated

17 years ago
Whiteboard: 1 day, eta 6/15.
(Assignee)

Comment 3

17 years ago
Created attachment 38261 [details] [diff] [review]
proposed fix
(Assignee)

Comment 4

17 years ago
the above fix basically just changes the unassert notifications to a series of
three unassert notifications

also, I had to move the unassertions BEFORE CutAllColumns() so that we could
still retrieve the row value

http://lxr.mozilla.org/seamonkey/source/xpfe/components/history/src/nsGlobalHistory.cpp#2915
(Assignee)

Comment 5

17 years ago
ok, that isn't quite working... mork appears to be trying hold onto the row
during the deletion...
(Assignee)

Comment 6

17 years ago
Created attachment 38269 [details] [diff] [review]
better fix - handles mork better
(Assignee)

Comment 7

17 years ago
so it turns out that mork was holding a weak reference to the row in the store -
so as long as I held a strong referene to it in nsGlobalHistory.cpp, the row
appeared to exist in the store. So basically, in addition to the above changes:
1) changed the semantics of FindRow() to always return an error condition when
the row is not found, and then updated the callers. Previously, it always
returns NS_OK, on success or failure.
2) change FindRow so that after finding the row in the store, that it is in the
main table, mTable. No, there is no way to call a FindRow directly on a table.

while working on this, I found some slowness in HasAssertion() where we fall
back to the GetTargets() case too often.... may help with the history perf bug.

this bug works beautifully, and allows the bulk-delete function to remove items
from the UI at the time they are deleted.
Status: NEW → ASSIGNED
Whiteboard: 1 day, eta 6/15. → fix in hand, waiting for r/sr/a

Comment 8

17 years ago
r/sr = bienvenu

Comment 9

17 years ago
Looks ok to me. sr=waterson

Comment 10

17 years ago
a= asa@mozilla.org for checkin to the trunk.
(on behalf of drivers)
Blocks: 83989
(Assignee)

Updated

17 years ago
Whiteboard: fix in hand, waiting for r/sr/a → fix in hand, landing 6/14
(Assignee)

Comment 11

17 years ago
yay, fix checked in. thanks all.
Status: ASSIGNED → RESOLVED
Last Resolved: 17 years ago
Resolution: --- → FIXED

Comment 12

17 years ago
I think it'd be mean to reopen this bug but I cannot verify it while bug 11412 is open, so I'll just
mark it depends.
Depends on: 11412

Updated

17 years ago
Whiteboard: fix in hand, landing 6/14 → waiting for 11412 before i can verify

Comment 13

17 years ago
VERIFIED Fixed with 2001070503 build
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.