Closed
Bug 636336
Opened 14 years ago
Closed 14 years ago
img/video/audio/source.setAttribute()/getAttribute() on src trims whitespace
Categories
(Core :: DOM: Core & HTML, defect, P2)
Core
DOM: Core & HTML
Tracking
()
RESOLVED
FIXED
mozilla5
People
(Reporter: ayg, Assigned: bzbarsky)
Details
Attachments
(1 file, 1 obsolete file)
Test case:
data:text/html,<!doctype html>
<script>
var img = document.createElement("img");
img.setAttribute("src", " ");
alert(img.getAttribute("src").length);
</script>
Alerts 0 in Firefox 4b11, 1 in Chrome 10 dev and Opera 11. Should clearly be 1 -- setAttribute() followed by getAttribute() should always return exactly the same thing no matter what.
(found with my reflection tests: http://dvcs.w3.org/hg/html/raw-file/tip/tests/submission/AryehGregor/reflection/reflection-original.html)
|
Assignee | |
Comment 1•14 years ago
|
||
Relevant hunk from nsHTMLImageElement::ParseAttribute:
372 if (aAttribute == nsGkAtoms::src) {
373 static const char* kWhitespace = " \n\r\t\b";
374 aResult.SetTo(nsContentUtils::TrimCharsInSet(kWhitespace, aValue));
375 return PR_TRUE;
376 }
Presumably that trimming is supposed to happen at some point, right? If not at attr set time, then where? Or is this not supposed to happen at all?
|
||
Comment 2•14 years ago
|
||
I guess this is supposed to happen somewhere in the "resolve an address" algorithm, but AFAIK, the IETF hasn't produced a useful spec yet.
|
|
Assignee | |
Comment 4•14 years ago
|
||
Also looks like nowadays net_FilterURIString handles this sort of thing. So I think we can just remove this code.
|
|
Assignee | |
Comment 5•14 years ago
|
||
Attachment #514780 -
Flags: review?(jonas)
|
|
Assignee | |
Updated•14 years ago
|
Assignee: nobody → bzbarsky
Priority: -- → P2
Whiteboard: [need review]
Attachment #514780 -
Flags: review?(jonas) → review+
|
|
Assignee | |
Comment 6•14 years ago
|
||
Er, wait. This is about more than just images.... Should have read the summary.
|
|
Assignee | |
Updated•14 years ago
|
Attachment #514780 -
Attachment is obsolete: true
|
|
Assignee | |
Comment 7•14 years ago
|
||
Attachment #515809 -
Flags: review?(jonas)
Comment on attachment 515809 [details] [diff] [review]
Don't mess with whitespace in the src attribute of images, now that necko can deal with it itself.
Please also test that something like
elem.setAttribute("src", " test ");
works as expected. Even better would be to also check that it gets reflected properly in .src (where the whitespace should be removed, right?)
Attachment #515809 -
Flags: review?(jonas) → review+
|
Reporter | |
Comment 9•14 years ago
|
||
Yes, reflecting is supposed to resolve the contents and return an absolute URL:
http://www.whatwg.org/specs/web-apps/current-work/multipage/urls.html#reflecting-content-attributes-in-idl-attributes
Actually I'm not sure it's currently specced anywhere how you're supposed to resolve a URL, but any sane web algorithm for that will involve stripping whitespace fairly early on, I imagine.
|
|
Assignee | |
Comment 10•14 years ago
|
||
> elem.setAttribute("src", " test ");
Added that test.
> Even better would be to also check that it gets reflected properly in .src
Added that too.
Whiteboard: [need review] → [need gk2 ship]
|
|
Assignee | |
Comment 11•14 years ago
|
||
Status: NEW → RESOLVED
Closed: 14 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Whiteboard: [need gk2 ship]
Target Milestone: --- → mozilla2.2
You need to log in
before you can comment on or make changes to this bug.
Description
•