Closed
Bug 1299373
Opened 9 years ago
Closed 9 years ago
Don't execute scripts with src=""
Categories
(Core :: DOM: Core & HTML, defect)
Core
DOM: Core & HTML
Tracking
()
RESOLVED
FIXED
mozilla51
| Tracking | Status | |
|---|---|---|
| firefox51 | --- | fixed |
People
(Reporter: bzbarsky, Assigned: bzbarsky)
References
Details
Attachments
(1 file, 2 obsolete files)
|
4.75 KB,
patch
|
bkelly
:
review+
|
Details | Diff | Splinter Review |
Per spec these should fire error and not execute. That's what Safari and Chrome seem to do, at least.
| Assignee | ||
Comment 1•9 years ago
|
||
Attachment #8786614 -
Flags: review?(bkelly)
| Assignee | ||
Comment 2•9 years ago
|
||
Attachment #8786645 -
Flags: review?(bkelly)
| Assignee | ||
Updated•9 years ago
|
Attachment #8786614 -
Attachment is obsolete: true
Attachment #8786614 -
Flags: review?(bkelly)
| Assignee | ||
Comment 3•9 years ago
|
||
Anne, Ben noticed an interesting issue while reviewing this patch. I had stopped using GetSrc because it doesn't return "" when the attr value is "" in Gecko. Per the spec, I _think_ this case should fall into the "If a reflecting IDL attribute is a USVString attribute whose content attribute is defined to contain a URL and is also required to contain a valid non-empty URL potentially surrounded by spaces" case and hence return "" if the attr value is "", right? But I don't see any browsers doing that. Gecko/Blink/WebKit all return the base URI; Edge returns the base URI with everything after the last '/' stripped off. Testcase at http://software.hixie.ch/utilities/js/live-dom-viewer/?saved=4429
Do you happen to know whether other engines are planning to align with the spec here or what the story is?
Flags: needinfo?(annevk)
Comment 4•9 years ago
|
||
The substeps of step 18 at https://html.spec.whatwg.org/#prepare-a-script do not use the src IDL attribute. They talk about the src content attribute. So you want the equivalent of getAttributeNS(null, "src") I'd say.
Flags: needinfo?(annevk)
Comment 5•9 years ago
|
||
Okay, I see now you're asking a question unrelated to OP. Simon looked into reflect a while back. I'm not sure if he covered this particular case, but that does seem like something that should maybe be changed.
Flags: needinfo?(zcorpan)
| Assignee | ||
Comment 6•9 years ago
|
||
> Okay, I see now you're asking a question unrelated to OP.
Not entirely. If the src IDL attribute actually behaved as currently specced, we _could_ use it in step 18. That's the relationship.
Comment 7•9 years ago
|
||
Comment on attachment 8786645 [details] [diff] [review]
Scripts with an empty src attribute should not execute
Dropping this for now until we have a plan.
Attachment #8786645 -
Flags: review?(bkelly)
| Assignee | ||
Comment 8•9 years ago
|
||
OK, filed https://github.com/whatwg/html/issues/1739 on the spec so I can move on here.
| Assignee | ||
Comment 9•9 years ago
|
||
Attachment #8787790 -
Flags: review?(bkelly)
| Assignee | ||
Updated•9 years ago
|
Attachment #8786645 -
Attachment is obsolete: true
Comment 10•9 years ago
|
||
Comment on attachment 8787790 [details] [diff] [review]
Scripts with an empty src attribute should not execute
Review of attachment 8787790 [details] [diff] [review]:
-----------------------------------------------------------------
Thanks!
Attachment #8787790 -
Flags: review?(bkelly) → review+
| Assignee | ||
Comment 11•9 years ago
|
||
Note that the spec has being reverted to have .src return something other than "" in this situation; I'll update the comment accordingly.
Comment 12•9 years ago
|
||
Pushed by bzbarsky@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/8edc64f3a6e4
Scripts with an empty src attribute should not execute. r=bkelly
Comment 13•9 years ago
|
||
| bugherder | ||
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla51
Updated•8 years ago
|
Flags: needinfo?(zcorpan)
Updated•6 years ago
|
Component: DOM → DOM: Core & HTML
You need to log in
before you can comment on or make changes to this bug.
Description
•