Last Comment Bug 366417 - Adding <html:script src="data:..."> to XUL document no longer executes the script
: Adding <html:script src="data:..."> to XUL document no longer executes the sc...
: fixed1.8.1.5, regression, testcase
Product: Core
Classification: Components
Component: DOM (show other bugs)
: Trunk
: PowerPC Mac OS X
: -- normal (vote)
: mozilla1.9alpha6
Assigned To: Jonas Sicking (:sicking)
: Hixie (not reading bugmail)
Depends on:
Blocks: 343730
  Show dependency treegraph
Reported: 2007-01-09 02:44 PST by Jesse Ruderman
Modified: 2008-10-17 15:53 PDT (History)
14 users (show)
jonas: blocking1.9+
jwalden+bmo: in‑testsuite?
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

testcase (339 bytes, application/vnd.mozilla.xul+xml)
2007-01-09 04:51 PST, Jesse Ruderman
no flags Details
Patch to fix (4.54 KB, patch)
2007-05-29 20:35 PDT, Jonas Sicking (:sicking)
no flags Details | Diff | Review
Patch v2 (16.82 KB, patch)
2007-05-30 13:00 PDT, Jonas Sicking (:sicking)
bzbarsky: review+
bzbarsky: superreview+
Details | Diff | Review
branch version (2.08 KB, patch)
2007-05-30 13:59 PDT, Jonas Sicking (:sicking)
bzbarsky: review+
bzbarsky: superreview+
dveditz: approval1.8.1.5+
dveditz: approval1.8.0.13-
Details | Diff | Review

Description Jesse Ruderman 2007-01-09 02:44:05 PST
When an <html:script> element with src="data:text/javascript,..." is added to a XUL document, it never executes.  When I do the same in an HTML document or with the src loaded from http, it works fine.

This regressed between 2006-11-03 and 2006-11-04.  Maybe it's a regression from bug 343730?
Comment 1 Jesse Ruderman 2007-01-09 04:51:38 PST
Created attachment 250951 [details]
Comment 2 Jesse Ruderman 2007-01-13 01:19:58 PST
This breaks a Greasemonkey script I rely on for testing Firefox.
Comment 3 Damon Sicore (:damons) 2007-05-29 11:46:09 PDT
Is this going to make A5?  If not, let's move please.
Comment 4 Jonas Sicking (:sicking) 2007-05-29 20:35:08 PDT
Created attachment 266569 [details] [diff] [review]
Patch to fix

This should fix it. The problem is that when we create the nsScriptLoader we're already inside a BeginUpdate, so the blockercount gets out of sync with the update level.

An alternative fix would be to make the nsDocument ctor create a scriptloader right away. Since nsContentSink forces creation of the scriptloader it happens most of the time anyway.

I'll attach a mochitest tomorrow.
Comment 5 Boris Zbarsky [:bz] 2007-05-29 21:10:15 PDT
I think eagerly creating the script loader is probably simpler conceptually (and may allow us to remove some null-checks too, right)?
Comment 6 Jonas Sicking (:sicking) 2007-05-30 13:00:11 PDT
Created attachment 266655 [details] [diff] [review]
Patch v2

Ok, this sets up the scriptloader in the ctor instead, and renames GetScriptLoader to ScriptLoader
Comment 7 Boris Zbarsky [:bz] 2007-05-30 13:11:22 PDT
Comment on attachment 266655 [details] [diff] [review]
Patch v2

Looks great!
Comment 8 Jonas Sicking (:sicking) 2007-05-30 13:59:55 PDT
Created attachment 266659 [details] [diff] [review]
branch version

Same as above, but without the /GetScriptLoader/ScriptLoader/ rename.
Comment 9 Jonas Sicking (:sicking) 2007-05-30 14:33:34 PDT
Patch checked in. Keeping open for a mochikit test to be written
Comment 10 Mike Connor [:mconnor] 2007-06-01 08:52:25 PDT
moving to a6 for the test to land
Comment 11 Nickolay_Ponomarev 2007-06-18 10:18:32 PDT
tweaking the summary to disambiguate from bug 304786.
Comment 12 Jonas Sicking (:sicking) 2007-06-20 11:01:56 PDT
Marking fixed, tracking the testcase using the flag instead.
Comment 13 Daniel Veditz [:dveditz] 2007-06-28 11:36:48 PDT
Comment on attachment 266659 [details] [diff] [review]
branch version

approved for and, a=dveditz for release-drivers
Comment 14 Adam Guthrie 2007-07-12 14:59:19 PDT
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv: Gecko/20070712 BonEcho/

Jonas landed this on branch on 2007-07-11 15:16, FWIW. 

That said, the testcase works for me in and in today's branch build, so marking as VERIFIED (since this works on trunk), but NOT adding the verified1.8.1.5 keyword.
Comment 15 Daniel Veditz [:dveditz] 2007-08-07 11:33:48 PDT
closing the tree for soon, will you be able to land this patch?
Comment 16 Daniel Veditz [:dveditz] 2007-08-09 11:33:11 PDT
Comment on attachment 266659 [details] [diff] [review]
branch version

not a blocker, misses the train

Note You need to log in before you can comment on or make changes to this bug.