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

VERIFIED FIXED in mozilla1.0.1

Status

()

Core
HTML: Parser
P2
major
VERIFIED FIXED
17 years ago
14 years ago

People

(Reporter: Marc 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)

4.41 KB, image/gif
Details
109 bytes, text/html
Details
5.97 KB, patch
John Keiser (jkeiser)
: review+
Judson Valeski
: approval+
Details | Diff | Splinter Review
(Reporter)

Description

17 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

17 years ago
CC'd pollmann and dougt since they both had patches for this in the other bugs

Comment 2

17 years ago
2 me
Assignee: neeti → dougt

Updated

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

Comment 3

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

Comment 4

17 years ago
Over to vishy@netscape.com.  He has 87298 and 23485 "whitespace" bugs.

Comment 5

17 years ago
.
Assignee: dougt → vishy
Status: ASSIGNED → NEW
-> alecf
Assignee: vishy → alecf
Target Milestone: mozilla0.9.3 → mozilla1.0.1

Comment 7

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

17 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

16 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

16 years ago
*** Bug 133536 has been marked as a duplicate of this bug. ***

Comment 16

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

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

Comment 20

16 years ago
Changed to P2 major.
Severity: normal → major
Priority: -- → P2

Comment 21

16 years ago
[adt2 RTM] as this issue is effecting top sites.
Blocks: 143047
Whiteboard: [bugscape 14779] → [bugscape 14779] [adt2 RTM] [TOPSITES]
(Assignee)

Comment 22

16 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

16 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.

Comment 24

16 years ago
applied patch v 1.0: 

displays images in patched build but not in 20020603/1.0.0

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

Comment 25

16 years ago
Created attachment 86366 [details]
mozilla-banner

Comment 26

16 years ago
Created attachment 86367 [details]
testcase
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

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

Addresses jst's concern.
(Assignee)

Updated

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

Comment 30

16 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

16 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

16 years ago
Keywords: adt1.0.1

Comment 34

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

Comment 35

16 years ago
Marking FIXED.
Status: ASSIGNED → RESOLVED
Last Resolved: 16 years ago
Resolution: --- → FIXED

Comment 36

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

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

Comment 38

16 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

16 years ago
Keywords: approval, mozilla1.0.1

Comment 39

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

Keywords: adt1.0.1 → adt1.0.1+

Updated

16 years ago
Attachment #86846 - Flags: approval+

Comment 40

16 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

16 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

16 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

16 years ago
verified fixed with branch build 20020619 using winxp

Updated

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

Updated

16 years ago
Keywords: verified1.0.1

Comment 45

15 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.