Closed Bug 87894 Opened 19 years ago Closed 18 years ago

Leading and trailing whitespace should be trimmed from IMG src attribute value.

Categories

(Core :: DOM: HTML Parser, defect, P2, major)

defect

Tracking

()

VERIFIED FIXED
mozilla1.0.1

People

(Reporter: attinasi, Assigned: harishd)

References

()

Details

(Keywords: compat, topembed+, Whiteboard: [bugscape 14779] [adt2 RTM] [TOPSITES][fixed on the trunk 06/11 and branch 06/18])

Attachments

(3 files, 2 obsolete files)

Spin-off from bug 87298 and bug 79929, both of which document specific cases of
whitespace handling in URLs. There is a lot of relevant information and even
patches in those two bugs, including RFC quotes, testcases, and test URLs.
CC'd pollmann and dougt since they both had patches for this in the other bugs
2 me
Assignee: neeti → dougt
Status: NEW → ASSIGNED
Target Milestone: --- → mozilla0.9.3
*** Bug 83603 has been marked as a duplicate of this bug. ***
Over to vishy@netscape.com.  He has 87298 and 23485 "whitespace" bugs.
.
Assignee: dougt → vishy
Status: ASSIGNED → NEW
-> alecf
Assignee: vishy → alecf
Target Milestone: mozilla0.9.3 → mozilla1.0.1
*** Bug 92555 has been marked as a duplicate of this bug. ***
*** Bug 95794 has been marked as a duplicate of this bug. ***
This is properly a parser bug. Reassigning.
Assignee: alecf → harishd
Component: Networking → Parser
QA Contact: benc → moied
Summary: Whitespace (at least leading and trailing) should be trimmed from URLs in QuirksMode → Leading and trailing whitespace should be trimmed from CDATA attributes in quirks mode.
*** Bug 95754 has been marked as a duplicate of this bug. ***
I don't think the parser messes with attrubtes at all. More likely, the content
elements themselves have to do this, as they understand the semantics of the
attribute values. I'm not sure who would own this though...
Related bugs for whitespace stripping, Bug 101087 Marked dup of 95754 and bug
95754 is marked dup of this bug,
whitespace trimming must be done in HTML mode only. In XML mode whitespace must
nut be trimmed. In HTML mode it must be trimmed.
*** Bug 130124 has been marked as a duplicate of this bug. ***
Keywords: compat
*** Bug 133536 has been marked as a duplicate of this bug. ***
http://bugscape/show_bug.cgi?id=14779 tracks top sites which are affected by
this bug.  

What are the changes of getting this fixed for RTM and 1.0.1?
Keywords: nsbeta1
Whiteboard: [bugscape 14779]
Please note that as per the title, this should be a quirks-mode-only change. 
While the three sites listed in this bug are quirks, any of the bugscape sites 
using standards mode should be evangelized.
*** Bug 148474 has been marked as a duplicate of this bug. ***
Changed to P2 major.
Severity: normal → major
Priority: -- → P2
[adt2 RTM] as this issue is effecting top sites.
Blocks: 143047
Whiteboard: [bugscape 14779] → [bugscape 14779] [adt2 RTM] [TOPSITES]
After investigating the bug I belive that this bug should in fact be fixed in
|layout|.

Reasons are:
1) We can target the fix to a specific attribute. Whereas in parser it would be
a more generic fix and therefore could have an impact on the performance.
2) Setting attribute values via DOM will benifit from layout fix. That is, if
the bug is fixed in the parser then attribute values, with leading/trailing
whitespace, set via DOM will exhibit the same bug.
3) With a layout fix we don't need additional code to preserve view-source.
Attached patch patch v1.0 (obsolete) — Splinter Review
Strip leading/trailing space characters, in IMG src, in addition to stripping
\n\r\t.
Attached image mozilla-banner
Attached file testcase
Comment on attachment 86333 [details] [diff] [review]
patch v1.0

Since speed is really critical in the sink where this new method is now used
I'd suggest changing the aSet argument to a simple const char pointer in stead
of a const nsAString&. Make that change and I'll have one more look.
Attachment #86333 - Flags: needs-work+
Attached patch patch v1.1 (obsolete) — Splinter Review
Addresses jst's concern.
Attachment #86333 - Attachment is obsolete: true
Attached patch patch v1.2Splinter Review
Minor optimization
Attachment #86754 - Attachment is obsolete: true
Comment on attachment 86846 [details] [diff] [review]
patch v1.2

sr=jst
Attachment #86846 - Flags: superreview+
Comment on attachment 86846 [details] [diff] [review]
patch v1.2

r=jkeiser
Attachment #86846 - Flags: review+
Fix landed ( 06/11 ) on the trunk.
Status: NEW → ASSIGNED
Whiteboard: [bugscape 14779] [adt2 RTM] [TOPSITES] → [bugscape 14779] [adt2 RTM] [TOPSITES][fixed on the trunk 06/11]
Keywords: adt1.0.1
moied, can you verify this on the trunk?
Marking FIXED.
Status: ASSIGNED → RESOLVED
Closed: 18 years ago
Resolution: --- → FIXED
I can verify using 2002-06-13-08/win2k trunk that the test case and the three sites

http://mimimaternity.com/collection.asp?category_Name=Dresses+%2D+Spring&category_Id=3045&PageIsOn=1
http://www.atlantic-records.com
http://www.metlife.com/

now show the missing images. atlantic still has some differences with respect ot
IE  however they are due to nav4/ie4 coding and are evang fodder.

vtrunk
Marking VERIFIED per bclary's comment ( #36 ).
Status: RESOLVED → VERIFIED
Changed summary to reflect the specific problem.
Summary: Leading and trailing whitespace should be trimmed from CDATA attributes in quirks mode. → Leading and trailing whitespace should be trimmed from IMG src attribute value.
adding adt1.0.1+.  Please get drivers approval before checking in.

Keywords: adt1.0.1adt1.0.1+
Attachment #86846 - Flags: approval+
please checkin to the 1.0.1 branch. once there, remove the "mozilla1.0.1+"
keyword and add the "fixed1.0.1" keyword.
Fix landed ( 06/18 ) on the branch.
Keywords: fixed1.0.1
Whiteboard: [bugscape 14779] [adt2 RTM] [TOPSITES][fixed on the trunk 06/11] → [bugscape 14779] [adt2 RTM] [TOPSITES][fixed on the trunk 06/11 and branch 06/18]
Added topembed+ per Susie's suggestion - just so any Bugzilla queries looking
for "topembed+" can pick this one up.
Keywords: topembedtopembed+
verified fixed with branch build 20020619 using winxp
Keywords: mozilla1.0.1+
*** Bug 126352 has been marked as a duplicate of this bug. ***
Keywords: verified1.0.1
*** Bug 122728 has been marked as a duplicate of this bug. ***
You need to log in before you can comment on or make changes to this bug.