Closed Bug 201998 Opened 21 years ago Closed 21 years ago

[FIX]relative images won't display when someone sends you a page using File | Send Page

Categories

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

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

Tracking

()

Status:
VERIFIED FIXED
Milestone:
mozilla1.4beta

People

(Reporter: sspitzer, Assigned: bzbarsky)

References

(
URL
)

Details

(Keywords: regression)

Attachments

(1 file, 3 obsolete files)

Shuffle code about
7.82 KB, patch
john
: review+
jst
: superreview+
Details | Diff | Splinter Review 
relative images won't display when someone sends you a page using File | Send Page

the problem isn't File | Send Page (or message compose).

I think the problem is in nsImageLoadingContent.cpp

when we try to load the image, the base url for the image is the url for the
message (think, imap://), instead of the web page.

for example, try this:

load http://www.sspitzer.org/images/, see the sspitzer.ico image.

no, do file send page, and notice the image isn't there.

from view message source:

Return-Path: <sspitzer@netscape.com>
Received: from netscape.com ([10.169.192.62]) by judge.mcom.com
          (Netscape Messaging Server 4.15) with ESMTP id HDCKBY03.E2F for
          <sspitzer@netscape.com>; Mon, 14 Apr 2003 11:50:22 -0700 
Message-ID: <3E9B01EE.3020907@netscape.com>
Date: Mon, 14 Apr 2003 11:46:06 -0700
From: sspitzer@netscape.com (Seth Spitzer)
User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.4b) Gecko/20030410
X-Accept-Language: en-us, en
MIME-Version: 1.0
To: Seth Spitzer <sspitzer@netscape.com>
Subject: test 2
Content-Type: multipart/mixed;
 boundary="------------030104050302090509040502"

This is a multi-part message in MIME format.
--------------030104050302090509040502
Content-Type: text/html; charset=us-ascii
Content-Transfer-Encoding: 7bit

<!DOCTYPE html PUBLIC "-//W3C//DTD HTML 4.01 Transitional//EN">
<html>
<head>
  <meta http-equiv="Content-Type" content="text/html;charset=ISO-8859-1">
  <title></title>
</head>
<body>
<br>
<a href="http://www.sspitzer.org/images/">http://www.sspitzer.org/images/</a><br>

</body>
</html>

--------------030104050302090509040502
Content-Type: text/html;
 name="www.sspitzer.org/images/"
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
 filename="www.sspitzer.org/images/"
Content-Base: "http://www.sspitzer.org/images/"
Content-Location: "http://www.sspitzer.org/images/"

<img src="./sspitzer.ico">

--------------030104050302090509040502--

this used to work, I'm not sure when it regressed.  (maybe with fix for bug #83774?)

here's a stack trace to where we get the base url.

nsImageLoadingContent::ImageURIChanged(const nsACString & {...}) line 378
nsImageLoadingContent::ImageURIChanged(nsImageLoadingContent * const 0x0733a600,
const nsAString & {...}) line 345 + 22 bytes
nsHTMLImageElement::SetAttr(nsHTMLImageElement * const 0x0733a5e0, int 0,
nsIAtom * 0x00ee4a08, const nsAString & {...}, int 0) line 617
HTMLContentSink::AddAttributes(const nsIParserNode & {...}, nsIHTMLContent *
0x0733a5e0, int 0) line 882
SinkContext::AddLeaf(const nsIParserNode & {...}) line 1812 + 28 bytes
HTMLContentSink::AddLeaf(HTMLContentSink * const 0x06eeb6f8, const nsIParserNode
& {...}) line 3586 + 18 bytes
CNavDTD::AddLeaf(const nsIParserNode * 0x07339aa0) line 3697 + 25 bytes
CNavDTD::HandleDefaultStartToken(CToken * 0x07310238, nsHTMLTag eHTMLTag_img,
nsCParserNode * 0x07339aa0) line 1384 + 12 bytes
CNavDTD::HandleStartToken(CToken * 0x07310238) line 1758 + 20 bytes
CNavDTD::HandleToken(CNavDTD * const 0x070ae910, CToken * 0x07310238, nsIParser
* 0x058ac6b8) line 942 + 12 bytes
CNavDTD::BuildModel(CNavDTD * const 0x070ae910, nsIParser * 0x058ac6b8,
nsITokenizer * 0x0709da00, nsITokenObserver * 0x00000000, nsIContentSink *
0x06eeb6f8) line 511 + 20 bytes
nsParser::BuildModel(nsParser * const 0x058ac6b8) line 1898 + 34 bytes
nsParser::ResumeParse(int 1, int 0, int 1) line 1765 + 12 bytes
nsParser::OnDataAvailable(nsParser * const 0x058ac6bc, nsIRequest * 0x06f97908,
nsISupports * 0x07102634, nsIInputStream * 0x06ec856c, unsigned int 0, unsigned
int 11873) line 2429 + 21 bytes
nsDocumentOpenInfo::OnDataAvailable(nsDocumentOpenInfo * const 0x071397b0,
nsIRequest * 0x06f97908, nsISupports * 0x07102634, nsIInputStream * 0x06ec856c,
unsigned int 0, unsigned int 11873) line 243 + 46 bytes
nsMimeBaseEmitter::Complete(nsMimeBaseEmitter * const 0x070ae5b8) line 961 + 47
bytes
nsStreamConverter::OnStopRequest(nsStreamConverter * const 0x06ec8498,
nsIRequest * 0x06f97908, nsISupports * 0x00000000, unsigned int 0) line 1071
nsDocumentOpenInfo::OnStopRequest(nsDocumentOpenInfo * const 0x06ede230,
nsIRequest * 0x06f97908, nsISupports * 0x00000000, unsigned int 0) line 256
nsImapCacheStreamListener::OnStopRequest(nsImapCacheStreamListener * const
0x06ecf2c0, nsIRequest * 0x070a9400, nsISupports * 0x00000000, unsigned int 0)
line 7512 + 43 bytes
nsInputStreamPump::OnStateStop() line 471
nsInputStreamPump::OnInputStreamReady(nsInputStreamPump * const 0x070a9404,
nsIAsyncInputStream * 0x071fee44) line 324 + 11 bytes
nsInputStreamReadyEvent::EventHandler(PLEvent * 0x071fef04) line 117
PL_HandleEvent(PLEvent * 0x071fef04) line 659 + 10 bytes
PL_ProcessPendingEvents(PLEventQueue * 0x00f0b510) line 592 + 9 bytes
nsEventQueueImpl::ProcessPendingEvents(nsEventQueueImpl * const 0x00ee3ea0) line
387 + 12 bytes
nsWindow::DispatchPendingEvents() line 3802
nsWindow::ProcessMessage(unsigned int 514, unsigned int 0, long 4915370, long *
0x0012fc34) line 4162
nsWindow::WindowProc(HWND__ * 0x00960442, unsigned int 514, unsigned int 0, long
4915370) line 1448 + 27 bytes
USER32! 77e3a244()
USER32! 77e145e5()
USER32! 77e1a792()
nsAppShellService::Run(nsAppShellService * const 0x015b83c8) line 479
main1(int 2, char * * 0x00271b40, nsISupports * 0x00f56aa0) line 1271 + 32 bytes
main(int 2, char * * 0x00271b40) line 1650 + 37 bytes
mainCRTStartup() line 338 + 17 bytes
KERNEL32! 77ea847c()

bz, any ideas?
interesting.. i also noticed sometime recently (post image loading from content)
that if i am editing a document with relative image URLs and manually change one
of the images to point at an absolute URL, all of the relative images will show
the broken image icon.  not sure if this is related, but it sounds like it could be.

this header is probably being ignored somehow:

  Content-Base: "http://www.sspitzer.org/images/"
So... I assume a message with the exact same source loads the image in a build
pre-bug-83774?

I'll try to take a look tonight, when I get home, but it's sounding like we're
just setting the "wrong" base URI on the document...
> So... I assume a message with the exact same source loads the image in a build
> pre-bug-83774?

right, on 3-17, this works.

esther reports:  "This broke sometime between the 3-18trunk build and the
3-25trunk build."

(from http://bugscape/show_bug.cgi?id=23273)
OK, this broke between build 2003-03-18-05 and 2003-03-19-05.  This is mine.

I know what's up, even... silly base tags...
Assignee: sspitzer → bzbarsky
Component: Mail Window Front End → Parser
OS: Windows 2000 → All
Product: MailNews → Browser
Hardware: PC → All
Attached file non-mail HTML testcase (obsolete) — 

    
    

    
  
Comment on attachment 120519 [details]
non-mail HTML testcase

Ignore me; I just can't spell href...  I can't get this to fail in browser...

        Attachment #120519 -
        Attachment is obsolete: true

    
    

    
  
Comment on attachment 120519 [details]
non-mail HTML testcase

Er, I meant "can't spell 'src'".

    
    

    
  

      
      
      
      

        Attached patch
          
          
        Patch to fix (obsolete)
        — 
          — Splinter Review
      

    


  
In any case, this fixes the mailnews problem, and I think it's the right way to
go.  The idea is to wait till we have the _base_href bogo-attr set before
loading the image....

    
  
Status: NEW → ASSIGNED
Priority: -- → P1
Summary: relative images won't display when someone sends you a page using File | Send Page → [FIX]relative images won't display when someone sends you a page using File | Send Page
Target Milestone: --- → mozilla1.4beta

    
  

        Attachment #120535 -
        Flags: superreview?(jst)

        Attachment #120535 -
        Flags: review?(jkeiser)

    
    

    
  
Yuck, what a nasty mess. Is there really no better way to deal with this? Why
don't we set the base for the *whole* document that's included in an email
message to the page that was included in stead of slapping a bogus base
attribute on every image tag? Or something, there's gotta be a better way, or at
least I hope so. Anyone with me?

    
    

    
  
> Why don't we set the base for the *whole* document that's included in an email

It looks like mail just injects the <base> into the HTML stream and we end up
hitting the ugly code at 
http://lxr.mozilla.org/seamonkey/source/content/html/document/src/nsHTMLContentSink.cpp#4439

But we may hit that ugly code even in non-mail stuff (my inability to write such
a testcase notwithstanding...).  And we have to deal.

I suppose I could make the image content node call ImageURIChanged if the
_base_href bogo-attr changes.  This should hopefully not happen too much and
hence should not slow things down too much (it'll be slower because we'll try to
do two load requests).

    
    

    
  


  
How about this in stead, untested, but should fix the problem.

    
    

    
  
I considered doing that (and tested it, btw -- it does fix the problem).  If we
do that we should change it at other places in the sink too so that the base is
_always_ added before other attributes.

If you'd like, I could do a patch to that effect.

    
    

    
  
Yeah, that sounds good. Less code, and faster code too! :-)

    
  

        Attachment #120535 -
        Flags: superreview?(jst)

        Attachment #120535 -
        Flags: review?(jkeiser)

    
    

    
  


  

        Attachment #120535 -
        Attachment is obsolete: true

        Attachment #120651 -
        Attachment is obsolete: true

    
  

        Attachment #120660 -
        Flags: superreview?(jst)

        Attachment #120660 -
        Flags: review?(jkeiser)

    
    

    
  
Comment on attachment 120660 [details] [diff] [review]
Shuffle code about

sr=jst

        Attachment #120660 -
        Flags: superreview?(jst) → superreview+

    
    

    
  
Comment on attachment 120660 [details] [diff] [review]
Shuffle code about

Yes sir, that's my baby, no sir, don't mean maybe, yes sir, that's my baby
nooooooww.

        Attachment #120660 -
        Flags: review?(jkeiser) → review+

    
    

    
  
Fixed.
Status: ASSIGNED → RESOLVED
Closed: 21 years ago
Resolution: --- → FIXED

    
    

    
  
cool, the bi-weekly messages I get like this look good in my debug build.

but I'll leave it to esther / nbaca to verify, since they are the pros.

    
    

    
  
Using trunk build on 20030418 on winxp, macosx and linux the bi-weekly emails we
receive have images now.  I also tested Seth's original sceanrio and it has an
image in the mail message when I do the Send page with the link provided. 
Verified for mail.   

    
    

    
  
Note, in comment 7 I don't see images when clicking on the link.   Boris, let me
know if I should see images when clicking on anyone of those links in comments 5
6 or 7?

    
    

    
  
Esther, you should not.  The testcase is using "href" instead of "src" on the
<img>, which just won't work... ;)

    
    

    
  
Thanks, verified.
Status: RESOLVED → VERIFIED

          You need to log in
          before you can comment on or make changes to this bug.
        






  

    
  







  

    

      
Attachment

      

      
      
      
      
    

    

      

        

          

            
General

            

              Creator: 



            

            
Created: 

            
Updated: 

            
Size: