Closed Bug 382397 Opened 17 years ago Closed 15 years ago

fix nsNavHistoryContainerResultNode::ReverseUpdateStats() to not notify unsorted views

Categories

(Toolkit :: Places, defect)

defect
Not set
normal

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.
a) we query to the toolbar folder directly it's supposed to be null.
OS: Windows XP → All
Product: Firefox → Toolkit
QA Contact: places → places
Hardware: x86 → All
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
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 → ---
Status: REOPENED → NEW
this should be fixed by bug 498130, the code changed a bit, and we check mParent->mParent in ReverseUpdateStats
Status: NEW → RESOLVED
Closed: 16 years ago15 years ago
Depends on: 498130
Resolution: --- → FIXED
no more fixed due to backout
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
relanded
Status: REOPENED → RESOLVED
Closed: 15 years ago15 years ago
Resolution: --- → FIXED
Assignee: nobody → mano
on 1.9.2 too
You need to log in before you can comment on or make changes to this bug.