Marco Bonardo [:mak] Assignee
	Description • 14 years ago

we load an html chunk as snippet, but <script> parts of it are not run by innertHTML.
One of the possible approaches could be to relocate them though through DOM manipulation, we could then just load the html chunk and aboutHome will take care of activating the scripts.
Other approaches are possible like sending just js content and creating a script element that will have to inject content by itself. But so far this is the simplest one I've found.

	Marco Bonardo [:mak] Assignee
	Comment 1 • 14 years ago

Underlying bug is bug 155146, bug 395597 is related.
I have no idea if the above 'hack' of reinserting a script element is going to be removed in future versions (Don't see a reason though, unless specs don't specifically forbid it), we should maybe ask bz if there is a plan for that.

	Marco Bonardo [:mak] Assignee
	Comment 2 • 14 years ago

cc-ing Boris.

	Boris Zbarsky [:bzbarsky]
	Comment 3 • 14 years ago

Creating a new script and inserting it into the DOM is not going to stop working, if that's the question.

	Marco Bonardo [:mak] Assignee
	Comment 4 • 14 years ago

Boris, do you think there could be a cleaner way to do the above?

	Boris Zbarsky [:bzbarsky]
	Comment 5 • 14 years ago

Not as long as you're using innerHTML....

	Marco Bonardo [:mak] Assignee
	Comment 6 • 14 years ago

updated based on IRC comments

	Marco Bonardo [:mak] Assignee
	Comment 7 • 14 years ago

http://hg.mozilla.org/mozilla-central/rev/c7987be54663

	Les Orchard [:lorchard]
	Comment 8 • 14 years ago

This was marked fixed, but I might have found an issue. Noted in bug 592431, but here it is again for completeness' sake:

If I serve up content like this:

<div class="snippet"><script
type="text/javascript;version=1.8">alert("HELLO")</script><p>Sample content
#1</p></div>
<div class="snippet"><script type="text/javascript;version=1.8">alert("HELLO
AGAIN")</script><p>Sample content #2</p></div>

It seems to break with an error in my console, with the snippets never shown
and JS never executed:

aboutHome.js (line 174) "Node was not found" code: "8
snippetsElt.replaceChild(relocatedScript, elt); 

If I remove the wrapper <div>s, things seem to work. Seems like it requires
bare <script> tags, but not sure why it would make a difference after looking
at the code

	Marco Bonardo [:mak] Assignee
	Comment 9 • 14 years ago

it is looking for <script> tags in the first level of the injected code, ideally a single script can handle all the content and replacing all scripts recursively would be quite expensive.

	Les Orchard [:lorchard]
	Comment 10 • 14 years ago

(In reply to comment #9)
> it is looking for <script> tags in the first level of the injected code,
> ideally a single script can handle all the content and replacing all scripts
> recursively would be quite expensive.

Well, if we only ever want to serve up a single piece of content, that's okay.

But, I'm expecting we'll want the option to serve up a collection of content items assembled into a single response by the server, based on matches against client info in the URL. That means we're likely to have multiple script tags.

	Boris Zbarsky [:bzbarsky]
	Comment 11 • 14 years ago

> it is looking for <script> tags in the first level of the injected code,

For what it's worth, the attached patch finds all, <script> tags that are descendants of snippetsElt, not just its kids.  The real issue is that this call:

  snippetsElt.replaceChild(relocatedScript, elt);

throws when relocatedScript is not a child of snippetsElt.  If you just replaced that line with:

  relocatedScript.parentNode.replaceChild(relocatedScript, elt);

it would all work.

	Marco Bonardo [:mak] Assignee
	Comment 12 • 14 years ago

uh, that's right, I'll file a bug apart, thanks for the hint.