Twisty icon remains after deleting all its children

RESOLVED FIXED

Status

()

RESOLVED FIXED
16 years ago
10 years ago

People

(Reporter: yuanyi21, Assigned: yuanyi21)

Tracking

Trunk
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

16 years ago
Steps:
1. open manage bookmars
2. create a folder named "a"
3. create a folder named "b" under "a"
4. remove b

actual result: "a" still has a twisty icon.

expected result: twisty icon should disappear.

same problem in addressbook.
(Assignee)

Comment 1

16 years ago
Created attachment 94310 [details] [diff] [review]
proposed patch

Updated

16 years ago
Keywords: patch
Jan, could you spare some to review this?
Keywords: nsbeta1, review
I see this in two other places. Will this patch address those as well?

1) Have a MailNews window open in threaded news connected to an IMAP server.
Delete the children of a threaded message with another IMAP client (maybe moz
running on a different computer, maybe a web client, etc). Observe that even
though there is only one message left in the "thread" a non-functional twisty
remains behind.

2) Read news in Show Only Unread mode with threads turned on. Read all the
messages in the thread. Collapse all threads "/" Observe there is one read
message with a non-functional twisty left over in the thread.
adt: nsbeta1-
Keywords: nsbeta1 → nsbeta1-
Attachment #94310 - Flags: review?(varga)

Comment 5

15 years ago
Eric, this patch only affects default views; the thread pane is a custom view
and many issues it has are specific to that implementation.

Comment 6

15 years ago
Comment on attachment 94310 [details] [diff] [review]
proposed patch

Interestingly hidden just below the bottom of the patch is a comment suggesting
almost exactly what is done, although the code would have to change slightly if
it was to be placed there. Also you probably only need to invalidate the parent
row's primary cell (although you probably copied that mistake from somewher
else). And you misspelled the first "Its". As for the first change, it looks
right to make the test match the comment. Note again that this test is done
after the content is removed.
(Assignee)

Comment 7

15 years ago
Neil, thanks for the review. This patch is over a year, I can not recall the
detail of those tricky code. Feel free to take it.
(Assignee)

Comment 8

15 years ago
ccing neil.

Comment 9

15 years ago
Comment on attachment 94310 [details] [diff] [review]
proposed patch

The attachment is incorrect, it should be clearing the Fill flag, not the other
two, anyway I tried this on bookmarks and it failed because of a bug in
bookmarks, and I spent ages hunting it down :-(
Attachment #94310 - Flags: review?(varga) → review-

Comment 10

15 years ago
Created attachment 131894 [details] [diff] [review]
Corrected patch
Attachment #94310 - Attachment is obsolete: true

Updated

15 years ago
Attachment #131894 - Flags: review?(varga)

Comment 11

15 years ago
Comment on attachment 131894 [details] [diff] [review]
Corrected patch

This looks much cleaner
r=varga
Attachment #131894 - Flags: review?(varga) → review+

Updated

15 years ago
Attachment #131894 - Flags: superreview?(dveditz+bmo)
*** Bug 88273 has been marked as a duplicate of this bug. ***
*** Bug 73827 has been marked as a duplicate of this bug. ***
*** Bug 221263 has been marked as a duplicate of this bug. ***
The nsTreeRows.h and nsXULTreeBuilder.cpp changes seem pretty straightforward --
what does the nsXULContentBuilder.cpp change fix?

Comment 16

15 years ago
The comment correctly describes what we want to happen i.e. always call
SetContainerAttrs. Perhaps if I quote from nsXULContentBuilder.cpp?
        if (aNewMatch) {
            // If there's no new match, then go ahead an update the
            // container attributes now.
            SetContainerAttrs(content, aOldMatch);
        }
    }

    if (aNewMatch) {
        // Get the content node to which we were bound
        Value value;
        PRBool hasassignment =
            aNewMatch->mAssignments.GetAssignmentFor(mContentVar, &value);

        NS_ASSERTION(hasassignment, "no content assignment");
        if (! hasassignment)
            return NS_ERROR_UNEXPECTED;

        nsIContent* content = VALUE_TO_ICONTENT(value);

        // Update the 'empty' attribute. Do this *first*, because
        // we may decide to nuke aNewMatch in a minute...
        SetContainerAttrs(content, aNewMatch);
Attachment #131894 - Flags: superreview?(dveditz+bmo) → superreview?(bryner)
Attachment #131894 - Flags: superreview?(bryner) → superreview+

Comment 17

15 years ago
Fix checked in.
Status: NEW → RESOLVED
Last Resolved: 15 years ago
Resolution: --- → FIXED
*** Bug 117856 has been marked as a duplicate of this bug. ***
*** Bug 148538 has been marked as a duplicate of this bug. ***

Comment 20

15 years ago
*** Bug 199352 has been marked as a duplicate of this bug. ***

Comment 21

15 years ago
*** Bug 217056 has been marked as a duplicate of this bug. ***

Comment 22

13 years ago
*** Bug 122075 has been marked as a duplicate of this bug. ***

Updated

10 years ago
Component: XP Toolkit/Widgets: Trees → XUL
QA Contact: shrir → xptoolkit.widgets
You need to log in before you can comment on or make changes to this bug.