Closed Bug 367400 Opened 17 years ago Closed 7 years ago

Add xblEnteredDocument/xblLeftDocument equivalents for XBL1

Categories

(Core :: XBL, defect)

defect
Not set
normal

Tracking

()

RESOLVED WONTFIX

People

(Reporter: peterv, Assigned: peterv)

References

(Blocks 2 open bugs, )

Details

Attachments

(2 files)

See bug 266590 for cases where we need it. We probably want to have elements like we have for constructor/destructor. Any suggestions for the names, or do people not find enteredDocument/leftDocument odd as method names?
<bind>...</bind> and <unbind>...</unbind> perhaps?
Blocks: 369573
Attached patch patchSplinter Review
Attachment #265260 - Flags: superreview?(bzbarsky)
Attachment #265260 - Flags: review?(peterv)
Attached file test
Comment on attachment 265260 [details] [diff] [review]
patch

I see several problems with this patch:

1)  Executing script during LoadBindings is a good way to crash exploitably
2)  Executing script during BindToTree (a.k.a. nsXBLBinding::ChangeDocument in
    this case), is a good way to crash exploitably.
3)  This doesn't handle the case of a binding being attached to a node that's not
    in a document and then that node actually being put in a document, which is
    the whole point of this bug.
Attachment #265260 - Flags: superreview?(bzbarsky)
Attachment #265260 - Flags: superreview-
Attachment #265260 - Flags: review?(peterv)
Attachment #265260 - Flags: review-
Oh, and the test should test for issue 3, of course.
(In reply to comment #4)
> (From update of attachment 265260 [details] [diff] [review])
> I see several problems with this patch:
> 
> 1)  Executing script during LoadBindings is a good way to crash exploitably
> 2)  Executing script during BindToTree (a.k.a. nsXBLBinding::ChangeDocument in
>     this case), is a good way to crash exploitably.

Any idea where the script should be executed?

> 3)  This doesn't handle the case of a binding being attached to a node that's
> not
>     in a document

How can it be done?

Ideally, the same place as we do other scripting stuff -- at the end of the update.  So keep track of what things need to be fired, and fire them when the update ends.

But note that you probably want to prevent out-of-order notifications, so might want to not fire if the notification doesn't match the current state.

> How can it be done?

cloneNode on a XUL element in a XUL document, where the thing you're cloning has a binding attached to it.
(In reply to comment #7)
> Ideally, the same place as we do other scripting stuff -- at the end of the
> update.

Boris, can you point me the code?
nsDocument::EndUpdate.
(In reply to comment #4)
> (From update of attachment 265260 [details] [diff] [review])
> I see several problems with this patch:
> 
> 2)  Executing script during BindToTree (a.k.a. nsXBLBinding::ChangeDocument in
>     this case), is a good way to crash exploitably.

nsXBLBinding::ChangeDocument is called from UnbindToTree and currently it isn't called from BindToTree. I guess I should call xbl:unbind before binding will be destroyed therefore it looks nsDocument::EndUpdate is not suitable place for this. Right? Should I call xbl:unbind from nsXBLBinding::ChangeDocument if document is empty?
Running script during UnbindFromTree is also unsafe.

If you need to run script before binding destruction, and binding destruction currently happens from UnbindFromTree, then you need to change it to happen later.
(In reply to comment #11)
> Running script during UnbindFromTree is also unsafe.
> 
> If you need to run script before binding destruction, and binding destruction
> currently happens from UnbindFromTree, then you need to change it to happen
> later.
> 

binding desctruction is called from nsGlobalWindow::PostHandleEvent on NS_PAGE_UNLOAD event.

I need to call script before binding will be deattached from document (i.e. when bingding is alive yet). Boris what place is more appropriate for this if UnbindFromTree is not safe for script.
I wasn't talking about the <destructor> but about whatever happens when you detach from the tree.

As for when to do your code, I have no idea.  I'd have to trace through the RemoveChild() code to see what times might work, and you can do that as well as I can...
(In reply to comment #13)
> I wasn't talking about the <destructor> but about whatever happens when you
> detach from the tree.

So if it's not about <desctructor> then which script may be executed during binding destruction?

> As for when to do your code, I have no idea.  I'd have to trace through the
> RemoveChild() code to see what times might work, and you can do that as well as
> I can...
> 

I have not idea what rule should I follow to see it because I do not know well code I patch. Can you give me a hint?
Maybe I was unclear.  You're trying to run script on removal from document.  Why?  That is, why do binding destructors not suffice for this use case?  Answering that question will tell you how late it's ok to run this notification; it sounds like you want it to happen before "something", but you've never said what this something is.

As for the rest, I don't really have time to spend on this for the foreseeable future, sorry.  I can spot-check things for you, but not do the work of going through the code in detail and so forth.  If it somehow becomes a 1.9 blocker, please let me know.

As a general rule, by the way, firing script at any time other than when it can already fire (e.g. when we fire mutation events or off the main event loop) is unsafe.
(In reply to comment #15)
> Maybe I was unclear.  You're trying to run script on removal from document. 
> Why?

Because I need to execute script before binding will be deattached from document.

>  That is, why do binding destructors not suffice for this use case? 

If binding destruction is happen on removal from document too and I can run script there then it's good place with me.

> Answering that question will tell you how late it's ok to run this
> notification; it sounds like you want it to happen before "something", but
> you've never said what this something is.

In general I need run script after bound element is added to document and before bound element is removed from document. Did I get you correct?

> 
> As for the rest, I don't really have time to spend on this for the foreseeable
> future, sorry.  I can spot-check things for you, but not do the work of going
> through the code in detail and so forth.  If it somehow becomes a 1.9 blocker,
> please let me know.

I would love to see the bug 1.9 blocker. This bug will fix some bugs related with performance issue (for example, richlistbox bug 369573 and xforms bug 372197).
(In reply to comment #16)
> > Answering that question will tell you how late it's ok to run this
> > notification; it sounds like you want it to happen before "something", but
> > you've never said what this something is.
> 
> In general I need run script after bound element is added to document and
> before bound element is removed from document. Did I get you correct?

The "run before bound element is removed from document" is going to be tricky. Why do you need to do this? Not even XBL2 supports this, its xblLeftDocument is executed after the element has been removed from the document.

You'll have to do it at either of these two lines:
http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/content/base/src/nsGenericElement.cpp&rev=3.568&mark=2658,2666#2650

And you'll have to do it for the entire subtree that is removed, which does raise performance concerns.
(In reply to comment #17)
> (In reply to comment #16)
> > > Answering that question will tell you how late it's ok to run this
> > > notification; it sounds like you want it to happen before "something", but
> > > you've never said what this something is.
> > 
> > In general I need run script after bound element is added to document and
> > before bound element is removed from document. Did I get you correct?
> 
> The "run before bound element is removed from document" is going to be tricky.
> Why do you need to do this? Not even XBL2 supports this, its xblLeftDocument is
> executed after the element has been removed from the document.

I'm just confused why XBL2 wants to execute it after the element has been removed from the document. For example, when richlistitem is removed from document then it sould notify richlistbox for update. If richlistitem binding get notification after then it won't have any related richlistbox.
I agree, a very good case could be made for executing it before the removal, you should raise it with the W3C WAF work group
QA Contact: ian → xbl
Blocks: XBL2
Web Components \o/
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: