299231 - [FIX]xbl bound elements generated from innerHTML don't work very well

Status: RESOLVED FIXED

(Core :: XBL, defect, P1)

Description

[Martijn Wargers (dead)]

See upcoming tetscase.
I can't access properties, methods and so from xbl bounds elements generated
from innerHMTL. 
I can access them when generated from a regular dom method (appendChild).

Comment 1

[Martijn Wargers (dead)]

Attachment: xbl needed for testcase

Comment 2

[Martijn Wargers (dead)]

Attachment: testcase

Comment 3

[Martijn Wargers (dead)]

In the testcase, click on the "add xbl spans" button. 
I would expect that the result for all the three tests be "true" with a green
background-color.
The "alert(span.thing)" button should alert 3 times 'true'.

Comment 4

[Boris Zbarsky [:bzbarsky]]

I bet the problem is that we disable script execution on the script context
while setting innerHTML so that <script> nodes in it don't execute...

Perhaps we should just disable the scriptloader, not disable script in general?

Comment 5

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

Why do we do that? <script>s generated through .innerHTML should execute no?

Comment 6

[Martijn Wargers (dead)]

See bug 214874 for why <script>s generated through .innerHTML should not execute.

Comment 7

[Boris Zbarsky [:bzbarsky]]

Bug 116834 is what initially introduced the innerHTML not running scripts thing.

jst, was there a reason for disabling scripts on the context there instead of
just disabling the script loader?  I guess the loader disabling is not a
counter, but neither is the script context thing...

Oh, and the surroundContents behavior on that testcase is somewhat correct, per
spec -- the new node is inserted first, then the kids its surrounding are
inserted; the constructor fires when there are no child nodes.  So I would
expect to see the text we see, though I'm not sure yet why the background is red.

Comment 8

[Boris Zbarsky [:bzbarsky]]

Actually, the surroundContents behavior is completely correct -- that
constructor throws an exception, as it should.  I've filed bug 299741 on the
exception not making it to the JS console in a timely manner.

Comment 9

[Boris Zbarsky [:bzbarsky]]

Attachment: Proposed fix

Comment 10

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

Comment on attachment 188328 [details] [diff] [review]
Proposed fix

I'm still not convinced that disabling scripts in .innerHTML is the right thing
to do. But this is not the bug for that debate.

Comment 11

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 188328 [details] [diff] [review]
Proposed fix

Requesting 1.8b4 approval to allow XBL attached to nodes created via innerHTML
to run its constructor.

Comment 12

[Boris Zbarsky [:bzbarsky]]

Fixed.