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

VERIFIED FIXED in mozilla1.0.1

Status

()

P2
major
VERIFIED FIXED
18 years ago
15 years ago

People

(Reporter: attinasi, Assigned: harishd)

Tracking

({compat, topembed+})

Trunk
mozilla1.0.1
compat, topembed+
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

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

Attachments

(3 attachments, 2 obsolete attachments)

(Reporter)

Description

18 years ago
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.
(Reporter)

Comment 1

18 years ago
CC'd pollmann and dougt since they both had patches for this in the other bugs
2 me
Assignee: neeti → dougt

Updated

18 years ago
Status: NEW → ASSIGNED
Target Milestone: --- → mozilla0.9.3

Comment 3

18 years ago
*** 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. ***
(Reporter)

Comment 11

18 years ago
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...

Comment 12

17 years ago
Related bugs for whitespace stripping, Bug 101087 Marked dup of 95754 and bug
95754 is marked dup of this bug,

Comment 13

17 years ago
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

Comment 15

17 years ago
*** 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.

Comment 19

17 years ago
*** Bug 148474 has been marked as a duplicate of this bug. ***

Comment 20

17 years ago
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]
(Assignee)

Comment 22

17 years ago
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.
(Assignee)

Comment 23

17 years ago
Created attachment 86333 [details] [diff] [review]
patch v1.0

Strip leading/trailing space characters, in IMG src, in addition to stripping
\n\r\t.
Keywords: nsbeta1 → nsbeta1+
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+
(Assignee)

Comment 29

17 years ago
Created attachment 86754 [details] [diff] [review]
patch v1.1

Addresses jst's concern.
(Assignee)

Updated

17 years ago
Attachment #86333 - Attachment is obsolete: true
(Assignee)

Comment 30

17 years ago
Created attachment 86846 [details] [diff] [review]
patch v1.2

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+
(Assignee)

Comment 33

17 years ago
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]
(Assignee)

Updated

17 years ago
Keywords: adt1.0.1

Comment 34

17 years ago
moied, can you verify this on the trunk?
(Assignee)

Comment 35

17 years ago
Marking FIXED.
Status: ASSIGNED → RESOLVED
Last Resolved: 17 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
(Assignee)

Comment 37

17 years ago
Marking VERIFIED per bclary's comment ( #36 ).
Status: RESOLVED → VERIFIED
(Assignee)

Comment 38

17 years ago
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.

Updated

17 years ago
Keywords: approval, mozilla1.0.1

Comment 39

17 years ago
adding adt1.0.1+.  Please get drivers approval before checking in.

Keywords: adt1.0.1 → adt1.0.1+

Updated

17 years ago
Attachment #86846 - Flags: approval+

Comment 40

17 years ago
please checkin to the 1.0.1 branch. once there, remove the "mozilla1.0.1+"
keyword and add the "fixed1.0.1" keyword.
Keywords: mozilla1.0.1 → mozilla1.0.1+
(Assignee)

Comment 41

17 years ago
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]

Comment 42

17 years ago
Added topembed+ per Susie's suggestion - just so any Bugzilla queries looking
for "topembed+" can pick this one up.
Keywords: topembed → topembed+

Comment 43

17 years ago
verified fixed with branch build 20020619 using winxp

Updated

17 years ago
Keywords: mozilla1.0.1+
*** Bug 126352 has been marked as a duplicate of this bug. ***

Updated

17 years ago
Keywords: verified1.0.1

Comment 45

16 years ago
*** 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.