Closed
Bug 382397
Opened 17 years ago
Closed 15 years ago
fix nsNavHistoryContainerResultNode::ReverseUpdateStats() to not notify unsorted views
Categories
(Toolkit :: Places, defect)
Toolkit
Places
Tracking
()
RESOLVED
FIXED
Tracking | Status | |
---|---|---|
status1.9.2 | --- | beta1-fixed |
People
(Reporter: moco, Assigned: asaf)
References
Details
fix nsNavHistoryContainerResultNode::ReverseUpdateStats() to not notify unsorted views
spun off from bug #379591
mano wrote, about itemChanged(), how can aNode.parent be null?
Here's how it happens when updating livemarks:
places.dll!nsNavHistoryContainerResultNode::ReverseUpdateStats(int aAccessCountChange=-1) Line 530 C++
places.dll!nsNavHistoryContainerResultNode::RemoveChildAt(int aIndex=0, int aIsTemporary=0) Line 1375 C++
places.dll!nsNavHistoryFolderResultNode::OnItemRemoved(__int64 aItemId=53, __int64 aParentFolder=11, int aIndex=0) Line 3130 C++
places.dll!nsNavHistoryResult::OnItemRemoved(__int64 aItemId=53, __int64 aFolder=11, int aIndex=0) Line 3718 + 0x8e bytes C++
places.dll!nsNavBookmarks::RemoveItem(__int64 aItemId=53) Line 968 + 0xa7 bytes C++
places.dll!nsNavBookmarks::RemoveFolderChildren(__int64 aFolder=11) Line 1356 + 0x21 bytes C++
The livemark container (on the personal toolbar folder) has a mParent, and it is the toolbar folder (itemId = 4)
The personal toolbar folder's parent is null.
nsNavHistoryContainerResultNode::ReverseUpdateStats() calls ItemChanged() with the mParent (so the personal toolbar folder),
so aNode.parent in toolbar.xml's itemChanged() is is null.
A couple of questions / comments:
a) should the toolbar folder's parent be null, or should it be the root?
b) for the toolbar.xml view, we are not not sorted. so we don't need the back end to notify the view in nsNavHistoryContainerResultNode::ReverseUpdateStats().
perhaps part of the fix here is to fix is to not call ItemChanged() on the view if our sort type is SORT_BY_NONE.
Assignee | ||
Comment 1•17 years ago
|
||
a) we query to the toolbar folder directly it's supposed to be null.
Updated•16 years ago
|
OS: Windows XP → All
Product: Firefox → Toolkit
QA Contact: places → places
Hardware: x86 → All
Comment 2•16 years ago
|
||
from what i see in actual code ReverseUpdateStats notifies the view only if mParent is valid, and a time or visit_count resorting has happened.
Looks like this bug, as it is, is no more valid.
Status: NEW → RESOLVED
Closed: 16 years ago
Resolution: --- → INVALID
Comment 3•15 years ago
|
||
I misread this as a problem with mParent, but we notify itemChanged(mParent) and we practically use mParent->mParent. Sorry for bad interpretation.
Status: RESOLVED → REOPENED
Resolution: INVALID → ---
Updated•15 years ago
|
Status: REOPENED → NEW
Comment 4•15 years ago
|
||
this should be fixed by bug 498130, the code changed a bit, and we check mParent->mParent in ReverseUpdateStats
Comment 5•15 years ago
|
||
no more fixed due to backout
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Comment 6•15 years ago
|
||
relanded
Status: REOPENED → RESOLVED
Closed: 15 years ago → 15 years ago
Resolution: --- → FIXED
Assignee | ||
Updated•15 years ago
|
Assignee: nobody → mano
You need to log in
before you can comment on or make changes to this bug.
Description
•