161452 - Twisty icon remains after deleting all its children

Status: RESOLVED FIXED

(Core :: XUL, defect)

Description

[Kyle Yuan]

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.

Comment 1

[Kyle Yuan]

Attachment: proposed patch

Comment 2

[[e-mail] (gone - use [e-mail] instead)]

Jan, could you spare some to review this?

Comment 3

[Eric Vaandering (no email)]

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.

Comment 4

[Rafael Ebron (:rebron)]

adt: nsbeta1-

Comment 5

[[e-mail]]

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

[[e-mail]]

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.

Comment 7

[Kyle Yuan]

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.

Comment 8

[Kyle Yuan]

ccing neil.

Comment 9

[[e-mail]]

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 :-(

Comment 10

[[e-mail]]

Attachment: Corrected patch

Comment 11

[Jan Varga [:janv]]

Comment on attachment 131894 [details] [diff] [review]
Corrected patch

This looks much cleaner
r=varga

Comment 12

[Stephen Donner [:stephend] Not actively reading bugmail]

*** Bug 88273 has been marked as a duplicate of this bug. ***

Comment 13

[Stephen Donner [:stephend] Not actively reading bugmail]

*** Bug 73827 has been marked as a duplicate of this bug. ***

Comment 14

[Stephen Donner [:stephend] Not actively reading bugmail]

*** Bug 221263 has been marked as a duplicate of this bug. ***

Comment 15

[Brian Ryner (not reading)]

The nsTreeRows.h and nsXULTreeBuilder.cpp changes seem pretty straightforward --
what does the nsXULContentBuilder.cpp change fix?

Comment 16

[[e-mail]]

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);

Comment 17

[[e-mail]]

Fix checked in.

Comment 18

[Stephen Donner [:stephend] Not actively reading bugmail]

*** Bug 117856 has been marked as a duplicate of this bug. ***

Comment 19

[Stephen Donner [:stephend] Not actively reading bugmail]

*** Bug 148538 has been marked as a duplicate of this bug. ***

Comment 20

[Asa Dotzler [:asa]]

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

Comment 21

[Asa Dotzler [:asa]]

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

Comment 22

[OstGote!]

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