Closed Bug 366417 Opened 13 years ago Closed 12 years ago

Adding <html:script src="data:..."> to XUL document no longer executes the script

Categories

(Core :: DOM: Core & HTML, defect)

PowerPC
macOS
defect
Not set

Tracking

()

VERIFIED FIXED
mozilla1.9alpha6

People

(Reporter: jruderman, Assigned: sicking)

References

Details

(Keywords: fixed1.8.1.5, regression, testcase)

Attachments

(3 files, 1 obsolete file)

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?
Attached file testcase
This breaks a Greasemonkey script I rely on for testing Firefox.
Flags: blocking1.9?
Flags: blocking1.9? → blocking1.9+
Assignee: general → jonas
Target Milestone: --- → mozilla1.9alpha5
Is this going to make A5?  If not, let's move please.
Attached patch Patch to fix (obsolete) — Splinter Review
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.
Attachment #266569 - Flags: superreview?(bzbarsky)
Attachment #266569 - Flags: review?(bzbarsky)
Attachment #266569 - Flags: superreview?(jst)
Attachment #266569 - Flags: superreview?(bzbarsky)
Attachment #266569 - Flags: review?(jst)
Attachment #266569 - Flags: review?(bzbarsky)
I think eagerly creating the script loader is probably simpler conceptually (and may allow us to remove some null-checks too, right)?
Attached patch Patch v2Splinter Review
Ok, this sets up the scriptloader in the ctor instead, and renames GetScriptLoader to ScriptLoader
Attachment #266569 - Attachment is obsolete: true
Attachment #266655 - Flags: superreview?(bzbarsky)
Attachment #266655 - Flags: review?(bzbarsky)
Attachment #266569 - Flags: superreview?(jst)
Attachment #266569 - Flags: review?(jst)
Comment on attachment 266655 [details] [diff] [review]
Patch v2

Looks great!
Attachment #266655 - Flags: superreview?(bzbarsky)
Attachment #266655 - Flags: superreview+
Attachment #266655 - Flags: review?(bzbarsky)
Attachment #266655 - Flags: review+
Attached patch branch versionSplinter Review
Same as above, but without the /GetScriptLoader/ScriptLoader/ rename.
Attachment #266659 - Flags: superreview?(bzbarsky)
Attachment #266659 - Flags: review?(bzbarsky)
Attachment #266659 - Flags: approval1.8.1.5?
Flags: in-testsuite?
Patch checked in. Keeping open for a mochikit test to be written
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Status: REOPENED → ASSIGNED
Attachment #266659 - Flags: superreview?(bzbarsky)
Attachment #266659 - Flags: superreview+
Attachment #266659 - Flags: review?(bzbarsky)
Attachment #266659 - Flags: review+
moving to a6 for the test to land
Target Milestone: mozilla1.9alpha5 → mozilla1.9alpha6
tweaking the summary to disambiguate from bug 304786.
Summary: Adding <script src="data:..."> to XUL document no longer executes the script → Adding <html:script src="data:..."> to XUL document no longer executes the script
Marking fixed, tracking the testcase using the flag instead.
Status: ASSIGNED → RESOLVED
Closed: 12 years ago12 years ago
Resolution: --- → FIXED
Comment on attachment 266659 [details] [diff] [review]
branch version

approved for 1.8.1.5 and 1.8.0.13, a=dveditz for release-drivers
Attachment #266659 - Flags: approval1.8.1.5?
Attachment #266659 - Flags: approval1.8.1.5+
Attachment #266659 - Flags: approval1.8.0.13+
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1.5pre) Gecko/20070712 BonEcho/2.0.0.5pre

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

That said, the testcase works for me in 2.0.0.4 and in today's branch build, so marking as VERIFIED (since this works on trunk), but NOT adding the verified1.8.1.5 keyword.
Status: RESOLVED → VERIFIED
closing the tree for 1.8.0.13 soon, will you be able to land this patch?
Comment on attachment 266659 [details] [diff] [review]
branch version

not a blocker, misses the train
Attachment #266659 - Flags: approval1.8.0.14?
Attachment #266659 - Flags: approval1.8.0.13-
Attachment #266659 - Flags: approval1.8.0.13+
Attachment #266659 - Flags: approval1.8.0.14?
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.