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

VERIFIED FIXED in mozilla1.9alpha6

Status

()

Core
DOM
VERIFIED FIXED
10 years ago
9 years ago

People

(Reporter: Jesse Ruderman, Assigned: sicking)

Tracking

({fixed1.8.1.5, regression, testcase})

Trunk
mozilla1.9alpha6
PowerPC
Mac OS X
fixed1.8.1.5, regression, testcase
Points:
---
Bug Flags:
blocking1.9 +
in-testsuite ?

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(3 attachments, 1 obsolete attachment)

(Reporter)

Description

10 years ago
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?
(Reporter)

Comment 1

10 years ago
Created attachment 250951 [details]
testcase
(Reporter)

Comment 2

10 years ago
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.
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.
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)?
Created attachment 266655 [details] [diff] [review]
Patch v2

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+
Created attachment 266659 [details] [diff] [review]
branch version

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
Last Resolved: 10 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

Comment 11

10 years ago
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
Last Resolved: 10 years ago10 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+
Keywords: fixed1.8.1.5

Comment 14

10 years ago
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?
You need to log in before you can comment on or make changes to this bug.