109200 - elements aren't properly removed when using the removeelement attribute

Status: RESOLVED FIXED

(Core :: Layout, defect, P3)

Description

[Scott MacGregor]

If you attempt to apply a removelement attribute to a xul element, we don't
properly unhook the element from the document before destroying it. According to
Chris this is because, nsXULDocument::RemoveElement needs to pass PR_TRUE as the
value of the aNotify parameter instead of PR_FALSE.

In my case, I was attempting to remove some command sets which were slowing me
down for message display. Changing this to TRUE did indeed improve matters as I
then started seeing calls to nsXULCommandDispatcher::RemoveCommandUpdater which
is what I expected. Although in my particular case, the updater wasn't properly
getting removed still. But that's a separate issue.

btw, I decided I don't need to use removeelement for my performance improvement
in 108761 so I'm not blocked on this particular change anymore. So check in the
change at your leisure Chris.

Comment 1

[Chris Waterson]

Attachment: noisy notification when removing content subtree

We need to do a noisy notify when removing subtrees here so that we properly
synchronize any crap that the document is observing (broadcasters, etc.)

Comment 2

[Chris Waterson]

Thanks for tracking this down, mscott.

Comment 3

[nnooiissee]

i am trying to edit the help menu for chatzilla, and have not been able to
remove menuitems (about plug-ins). have not been able to add them very well
either, but that is a story for a different bug.

hope to see this landed soon.

Comment 4

[Ben Goodger (use ben at mozilla dot org for email)]

Comment on attachment 57181 [details] [diff] [review]
noisy notification when removing content subtree

r=[e-mail]

what, me, buggy code? never!

;)

Comment 5

[David Hyatt]

Any perf implications here?  If not, sr=hyatt

Comment 6

[nnooiissee]

what's the state on this? it has an r, and a pending sr. have performance issues
been found, or is it waiting on someone to check?

can someone give a status update please?

Comment 7

[Chris Waterson]

Didn't read this carefully. I'll check this in today.

Comment 8

[Chris Waterson]

Fix checked in. Sorry I dropped the ball!

Comment 9

[nnooiissee]

thank you