Closed Bug 87894 Opened 24 years ago Closed 23 years ago

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

Categories

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

Product:
Core
Component:
DOM: HTML Parser
Type:
defect

Tracking

()

Status:
VERIFIED FIXED
Milestone:
mozilla1.0.1

People

(Reporter: attinasi, Assigned: harishd)

References

(
URL
)

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)

mozilla-banner
4.41 KB, image/gif
Details
testcase
109 bytes, text/html
Details
patch v1.2
5.97 KB, patch
john
: review+
jst
: superreview+
jud
: approval+
Details | Diff | Splinter Review 
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 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.2
        — 
          — Splinter 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: 23 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.1 → adt1.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.
Keywords: mozilla1.0.1 → mozilla1.0.1+

    
    

    
  
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: topembed → topembed+

    
    

    
  
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.
        






  

    
  







  

    

      
Attachment

      

      
      
      
      
    

    

      

        

          

            
General

            

              Creator: 



            

            
Created: 

            
Updated: 

            
Size: