229703 - I seem xbl is destroyed when I call removeChild and then appendChild

Status: NEW

(Core :: XUL, defect)

Description

[alexander :surkov (:asurkov)]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.5) Gecko/20031007
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.5) Gecko/20031007

When I have link to xbl element in JavaScript, then I remove it by removeChild()
method, and then I append to some DOM element by appendChild() method, then I
think xbl element is destroyed after call removeChild() method.

Reproducible: Always

Steps to Reproduce:
1. Cretate textbox and html:input elements, write some text there
2. Replace textbox and input in other place of DOM by calling removeChild() and
appendChild()

Actual Results:  
Text typed into textbox disapeared. Text typed into input was copied.

Expected Results:  
Text typed into textbox should was to be copied also.

Comment 1

[alexander :surkov (:asurkov)]

Attachment: test case

Type text into textfields and press button. Text typed into textbox will
disapear. Text typed into html:input will copied.

Comment 2

[Boris Zbarsky [:bzbarsky]]

When you take a node out of the DOM, its presentation, including any XBL binding
WILL be destroyed.  That's correct.

If state stored in that presentation needs to be preserved, the presentation
needs to take care of that.  The layout object for <html:input> does so.

In other words, the destructor for the textbox binding needs to save the value
on the node itself and the constructor needs to restore it as needed.

Not a DOM bug; over to XP Toolkit.

Comment 3

[[e-mail]]

<textbox> already has some code which looks as if it's trying to save/restore
the value using boxObject.[sg]etProperty - if that's wrong, where could the
value be saved?

Comment 4

[Boris Zbarsky [:bzbarsky]]

No, that looks right... What's the value of this.inputField.value in that
destructor?  What string comes out of the boxObject in the constructor?

Is the problem that the "value" inheritance is applied after the constructor
fires or something?

Comment 5

[[e-mail]]

A quick dump in the JS console shows that the textbox, having moved, now has a
new box object, so that all the old properties have been lost...

Comment 6

[Boris Zbarsky [:bzbarsky]]

Ah, indeed.  When nodes are removed from a document they wipe out their box
object (see nsXULElement::SetDocument and nsGenericElement::SetDocument).   So
storing state on the box object really doesn't work for nodes moving out of the
DOM then back in.  It works fine for nodes going to display:none and back.

jst, should we leave box objects around until the nodes are destroyed, maybe? 
It may make sense to clear the box object when:

1)  The node is destroyed
2)  The node is inserted into a new document (such that a new nodeinfo is
    needed)

in both cases, we would use the document in the old nodeinfo as the document in
which the box object lives...

Comment 7

[alexander :surkov (:asurkov)]

I think object should be destroyed when it isn't referenced. It is unimportant 
this object is lying into DOM or it is referenced from JavaScript. 
I refer to removed from DOM object by javascript variable. While this variable 
doesn't leave scope than removed from DOM object should not be destroyed.

Comment 8

[Boris Zbarsky [:bzbarsky]]

Alexander, what object are you talking about exactly?  The DOM node is NOT being
destroyed at any point here.  What is being destroyed is the node's presentation
(rendering objects), which DO need to be destroyed as soon as the node is
removed from the DOM (otherwise it would continue to render).

Comment 9

[alexander :surkov (:asurkov)]

Ok. I agree presentation object should be destroyed. But I can't understand why
should I be care for saving properties by boxObject as in the case of textbox?
And I have a question: Is boxObject DOM object, presentation (rendering object)
or something else? And why boxObject is destroyed when I remove element from DOM
if boxObject serves to save propeties.

Comment 10

[Boris Zbarsky [:bzbarsky]]

> But I can't understand why should I be care for saving properties by boxObject
> as in the case of textbox?

_You_ probably shouldn't.  The binding author should.  And I'm not sure the box
object is the right place to save it, truth to tell.  I would have saved it on
the node...

> And I have a question: Is box object DOM object, presentation (rendering
> object) or something else?

Almost all methods and properties on nsIBoxObject are presentation-dependent, so
I would say it's a presentation object...  But it's treated as a something else
for some unknown-to-me reasons.

> And why boxObject is destroyed when I remove element from DOM
> if boxObject serves to save propeties.

Because whoever wrote the binding code clearly wasn't talking to whoever wrote
the boxobject code....  I suspect they only tested using display:none, not
actually removing nodes from the DOM.

Comment 11

[alexander :surkov (:asurkov)]

If I understand right then DOM element isn't destroyed at all when I remove it
from DOM (probably DOM element will be destroyed later by garbage collector) and
it's presentation is destroyed only. Then why do xbl's properties belong to
presentation object but not to DOM object? Because if I change xbl's property
from JavaScript and move (remove/append) this xbl to other place of DOM then
xbl's property will be droped.
Futher, let properties belong to presentation object. Presentation of html:input
element serves value when it will be moved inside of DOM. Then why doesn't
presentaton of html:input contained into textbox serve value when it will be
moved inside of DOM?

Comment 12

[Boris Zbarsky [:bzbarsky]]

> Then why do xbl's properties belong to presentation object but not to DOM
> object?

Because XBL is a form of presentation -- the same DOM node could have different
XBL bindings bound to it in different parts of the DOM tree, for example....
just like CSS presentation.

I'm not sure what your last question is asking... the presentation object for
html:input makes sure to stash the value away on the DOM node when it's
destroyed so that the value survives destruction of presentation objects (eg so
that display:none forms can submit).  The textbox binding tries to save the
state as well, but saves in a less permanent place than the DOM node, hence the
issues described in this bug.

The more I think about it, the more I think textbox needs to be storing
properties on the domnode itself, not on the boxobject.

Comment 13

[Boris Zbarsky [:bzbarsky]]

Neil, if you have thoughts (even a clear explanation of what the lifetime of
boxobjects should be), please let me know....  (and document that in the IDL?)

Comment 14

[[e-mail]]

This problem cropped up with rdfliners... what used to happen was that when an
rdfliner was made visible its box object would create a selection object (which
holds a weak reference to the box object) and attach it to the treebuilder
object; if you then removed and readded the node you would delete the box object
thus invalidating the pointer in the selection object...

Any custom view is also set as a property on the box object, so that removing
and readding a tree will forget the custom view...

I don't like the idea of setting attributes on the DOM node so where else can
you suggest?

Comment 15

[Boris Zbarsky [:bzbarsky]]

> I don't like the idea of setting attributes on the DOM node

Why not?  You read any existing "value" attribute and use it as the initial
value anyway, no?

As for what else I can suggest, it's to make box objects a little more
persistent as per comment 6.  Note that state would still get lost if you moved
a node between documents, then... (boxobjects are very much per-document).

Comment 16

[[e-mail]]

The value is stored on the box object so that it will survive a dynamic theme
change (not that we support that any more...) - it could have been stored as an
attribute but it wasn't and I'm guessing there was a reason for that...

Comment 17

[Boris Zbarsky [:bzbarsky]]

Ah, I see.  In other words, the goal is NOT to handle the situation at hand, huh? ;)

Well, if the solution from comment 6 is desired at some point let me know.

Comment 18

[alexander :surkov (:asurkov)]

To comment <a
href="http://bugzilla.mozilla.org/show_bug.cgi?id=229703#c10">10</a>. I think
xbl author shouldn't to care for saving of properties when I set up property
display:none or move element to other place in DOM. Because it give discomofort
due development of xbl. I think it will be more comfortable If I will not take
care of properties saving and if it will be done automatically.
More. I don't understand how work saving of properties of textbox (comment <a
href="http://bugzilla.mozilla.org/show_bug.cgi?id=229703#c3">6</a>). Because xbl
destructor isn't called at all (see testcase of bug <a
href="http://bugzilla.mozilla.org/show_bug.cgi?id=230086">230086</a>).

Comment 19

[Michael Kraft [:morac]]

I'm noticing that "field" elements in a XBL element are reset back to their default values when the XBL element is removed from the DOM.  This differs from any manually set javascript variables which retain their values.  

Would that be covered under this bug or would it be a separate issue?