226016 - [FIX]Removing all children from a <button> and inserting new ones doesn't work

Status: RESOLVED FIXED

(Core :: Layout: Form Controls, defect, P2)

Description

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

For the following markup:

<button id="theButton">TextHere</button>

calling
button.removeChild(button.firstChild);
button.appendChild(document.createTextNode("TextHere");

puts the button in a weird state. Subsequent calls to either removeChild or
appendChild will trigger a "not a container" assertion in layout and the changes
will not be reflected in the rendering of the document.

Reframing the button will put it back in a fully functional state again. Until
the same operation is performed again (i.e. removing all children and putting a
new one in).

This happened on a customer webpage where the customer set .innerHTML twice on
the button (setting .innerHTML removes all children before inserting any new ones)

Testcase comming up.

Comment 1

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Attachment: testcase

An intersting sidenote is that the 'reframe' button doesn't work in optimized
builds. All it does is setting .style.display="none"; .style.display=""; on a
parent of the button.

Comment 2

[Boris Zbarsky [:bzbarsky]]

I bet this is the same thing as bug 98239 at heart.

Comment 3

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Yeah, that's definitly the same, feel free to dup if you want (though this one
has a higher signal/noice ratio).

I think what happens is that we have some sort of inner-frame for buttons that
acts as the block-frame, which only gets created during the initial reflow and
not when a node is inserted into an empty button

Comment 4

[Boris Zbarsky [:bzbarsky]]

Exactly.  Well, sorta.  I have a patch that makes things "work", but triggers a
different set of asserts... there's also some style context wackiness here that
I need to look into.

Comment 5

[Boris Zbarsky [:bzbarsky]]

Attachment: Wackiness and asserts were all the same deal

This fixes the testcase and all the testcases in bug 98239; no more asserts.

Comment 6

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 135769 [details] [diff] [review]
Wackiness and asserts were all the same deal

David, would you do the honors?

Comment 7

[Boris Zbarsky [:bzbarsky]]

.

Comment 8

[Boris Zbarsky [:bzbarsky]]

Fixed.