Open
Bug 62026
Opened 24 years ago
Updated 2 years ago
Inline images in news/imap messages not rendered progressively
Categories
(MailNews Core :: MIME, enhancement, P3)
MailNews Core
MIME
Tracking
(Not tracked)
NEW
Future
People
(Reporter: jg, Unassigned)
References
Details
(Keywords: perf, Whiteboard: [patchlove][has draft patch][needs owner])
Attachments
(2 obsolete files)
In Nav4.x, a really nice feature (and mentioned as such in mag reviews) is that if you're on a modem or slow link and you click on a news post that contains an image attachment, rather than wait for the entire post to be downloaded before rendering the image, the image slowly renders as it comes down. This is the same behaviour as with progressive jpeg/gifs in webpages, and can be extreemly useful if you don't want to download potentially 400k+ of image(s) when you could hit the stop button after seeing the top section of the image and save time. Granted, not the most serious of bugs, but it will be missed from Mozilla/Netscape 6 until it's done. Assigning to MailNews/Front End in the absence of any other clue about the owner. Also ccing pnunn as I'm told it could be an imagelib issue. Did look for existing bug on this but was unsuccessful. Note this only happens in MailNews, not browser, affects all builds to current (2000120508) Linux.
Reporter | ||
Comment 1•24 years ago
|
||
Nominating for mozilla0.9. Adding 4xp keyword.
Keywords: 4xp,
mozilla0.9
Comment 4•23 years ago
|
||
A usability problem for those of us that have a slow internet connection and download large images from newsgroups.
Comment 5•23 years ago
|
||
This would increase usability enormously as far as I'm concerned. Here, have another vote.
Reporter | ||
Comment 6•23 years ago
|
||
I would suggest 0.9.3 nomination, except we have no 'mozilla0.9.3' keyword. 1.0 it is then. Can someone suggest what needs to be done here? CC pavlov who might have a clue.
Keywords: mozilla1.0
Comment 7•23 years ago
|
||
Isn't it expensive to reflow each time you get yet another chunk of data to display, if doing it progressively?
Reporter | ||
Comment 8•23 years ago
|
||
No, in the image header you will find the properties for width and height; you then create an approriately sized frame (one reflow) and fill it with colour. Continue this for each image attachment.
Comment 9•23 years ago
|
||
Then I think this is something that should be in libpr0n code. probably saari's or pavlov's.
Comment 10•23 years ago
|
||
the image should be getting flowed at its size pretty early. as for it drawing progressivly, it should, but i wouldn't be too suprised if it didn't. there could be a problem with the uriloader code that does the conversion between the news: url to pull the image data out... it could be doing this all syncronously and not passing data to imagelib in chunks. cc'ing mscott
Comment 11•23 years ago
|
||
It looks like the whole kaboodle is only being rendered in just one shot, since if there is a textual pre-amble and then an image you don't even get to see the preamble until both the text AND the image are downloaded. i.e. it's not (solely) an image-display problem.
Comment 12•23 years ago
|
||
This feature is also missing in the Windows version
Comment 13•23 years ago
|
||
This feature is also missing in the Windows Version
Updated•23 years ago
|
Blocks: advocacybugs
Comment 14•23 years ago
|
||
If so, it should be All/All. (I have too fast access to my News serverto be sure). Marking.
OS: Linux → All
Hardware: PC → All
Comment 15•23 years ago
|
||
This may be a related issue, please comment before I open another bug: http://www.fuckedcompany.com/images/911/5th_avenue_facing_south.jpg Clear your cache before loading above image. This is a good sized jpg which should progressivly load as data is received, but it is only displayed after it's completely received.
Comment 16•23 years ago
|
||
This doesn't look like a news bug, it happens in browser as well. Reassign to imglib, redired if I'm wrong.
Assignee: sspitzer → pavlov
Component: Mail Window Front End → ImageLib
Product: MailNews → Browser
QA Contact: pmock → tpreston
Comment 17•23 years ago
|
||
this is a dup of the "jpegs dont' render progressivly" bug..
Comment 18•23 years ago
|
||
lophat's example is indeed a dup, pavlov, but isn't what THIS bug is about.
Comment 19•23 years ago
|
||
Can we agree, since the non-image preamble to posts doesn't render even while we're waiting for the image, that this is not strictly an imagelib problem? ... and in that case, reassign it (ie. to the MailNews module owner)?
Comment 20•23 years ago
|
||
Ahh, what the cripes. Cor lummy. product -> mailnews component -> networking (would like input from the net folk about where this data is probably logjamming)
Component: ImageLib → Networking - News
Product: Browser → MailNews
Target Milestone: --- → mozilla1.0
Updated•22 years ago
|
Target Milestone: mozilla1.0 → mozilla1.1
Comment 21•22 years ago
|
||
Eh, actually Mozilla already have this feature. When loading image, press stop button -> it takes few seconds and mozilla starts loading image again, but this time image is loaded progressively. So, *why* it's so hard to do that instantly?
Comment 22•22 years ago
|
||
This should give some insight into the bug (or spawn off another bug :-) : Subscribe to a newsgroup that typically deals with UUencoded binaries (either movies or warez, or anything else that is sent as large files *that are broken into parts*). At first you have not clicked on a message yet, so your message panel should be empty. Now find a binary that's broken into parts (ie 1/6, 2/6, ...) and click on any part *except for the first one*. What you'll see: First you will get the header info show up with absolutely no text underneath. Then a few seconds later you'll see *some* uuencoded text (it won't be decoded since it doesn't have the info to do that). Then, every few seconds, you'll get more uuencoded text until it's complete. My belief is that this is due to 1. The newsreader buffering too much of the message before updating the display 2. The newsreader flushing the buffer after getting the header information only (Which, on slow connections, means seeing the header and then waiting a few seconds before getting the text of long messages.) IANAC (I am not a coder) but it appears that this can be fixed by decreasing the buffer size and flushing it more often. And if the buffer size is small enough, images will display progressively (Getting back to the actual problem. :-)
Comment 23•22 years ago
|
||
From following your steps I'll agree that *something* is buffering too much, but that is only part of the problem because I can view postings with images which are hundreds of K in size and still not see a thing until the end, and the thought that something in mozilla could be buffering hundreds of K when my line speed is equivilent to a wet piece of string -- well, it sends shivers down my spine.
Comment 24•22 years ago
|
||
It just occurred to me how generous we're being with this bug's severity. If inline images in web pages all stopped rendering progressively I'd be surprised if the tree didn't get closed; why then allow the same problem to languish in mailnews? That mozilla1.0 keyword has also been there for quite a while; who should be CC'd to + or - it?
Comment 25•22 years ago
|
||
According to the Mozilla 1.0 manifesto, this is definitely not a Mozilla 1.0 blocker. However, I'm sure a patch would get into the trunk pretty quickly if you wrote it. ;-)
Comment 26•22 years ago
|
||
*fx: wind blows, tumbleweed rolls by*
Comment 27•22 years ago
|
||
You forgot: *fx: coyote howls in distance, add cricket and toad vocalizations to taste*
Comment 28•22 years ago
|
||
Adorable one-line fix. Too cute to be on the trunk, for a few reasons (including that depending on buffer sizes in various inpenetrable parts of mozilla, this might cause sucky CPU usage on fast connections, though will likely improve percieved speed -- flushes should probably be on a timer like the browser's doc reflows). Also fixes the same incremental rendering problem over IMAP and the incremental-text problem described in bug 135925
Comment 29•22 years ago
|
||
I verify that this causes CPU/responsiveness suckage from news/mail cache, so I expect the same problem from high-speed connections. Pretty predictable from the nature of the patch, but it does indicate where the problem roughly lies.
Comment 30•22 years ago
|
||
I tell a lie, the CPU suckage when loading from cache was rather due to my layout prefs. OnWrite() calls and hence the subsequent OnFull() flushes are actually getting coelesced fairly well when loading from a fast input. I don't know why, without the patch, OnFull() is not getting called until the input stream finishes. That's the crux of the bug. I can't imagine that there really is hundreds of KB of buffer at this level (mBufferMgr's allocated hunk size doesn't ever get above about 150 bytes here). Interestingly, if I don't do the OnFull() flush from OnWrite() then OnWrite() just stops being called quite soon into the load, never to be called again until the input ends and gets its final completion-flush. Something broken in the buffering logic? Or is nsMimeBaseEmitter /supposed/ to be pretty synchronous and hence this patch is more or less the correct thing to do, and its omission the bug? Who knows nsMimeBaseEmitter? sspitzer?
Comment 31•22 years ago
|
||
Anyone...?
Comment 32•22 years ago
|
||
Amending the summary since the symptoms (and apparent fix) are exactly the same when reading remote email over IMAP.
Summary: Inline images in news posts not rendered progressively → Inline images in news/imap messages not rendered progressively
Comment 33•22 years ago
|
||
If it turns out that the submitted patch is conceptually pretty much right then I'd be willing to un-cute it (factor out the flushing to a private function that gets called from OnWrite() and OnFull() [and possibly Complete()], etc)
Comment 34•22 years ago
|
||
since it's mime, ducarroz would be the person to ask for review. removing TM, since 1.1 alpha has passed.
Assignee: pavlov → ducarroz
Target Milestone: mozilla1.1alpha → ---
Comment 35•22 years ago
|
||
My concern is that mime decodes the stream line per line and therefore nsMimeBaseEmitter::OnWrite can be called very often (with only couple bytse of data to write), several hundred time per message depending on the content. Therefore this patch might cause a performance degradation issue. We nee to call onFlush using a timer...
Comment 36•22 years ago
|
||
Thanks for looking at it! Yes, OnWrite occurs every fewscore decoded bytes, which is fairly likely to hurt on a faster connection. I'd rather see flushing happen on a timer too. Who should own the timer though? It doesn't feel right to put that stuff in the low-level emitters. Higher-level users of nsMimeBaseEmitter could individually poll the stream for new data to 'pull' on a timer, but... I still wonder why OnFull doesn't fire until completion, anyway. Any idea?
Comment 37•22 years ago
|
||
You are right, mime should own the timer. It's not the job of the emitter to do that! That should be trivial to add the timer in mime...
Comment 38•22 years ago
|
||
n.b. if anyone is waiting for me to handle this, I'm unlikely to have time for about a hundred years. At least hopefully we have a better idea now of where the problem lays and how to fix it (though I still fail to understand whether this is a real bug in that buffer-management is broken because flushes apparently never implicity happen, or just an oversight or bad assumption by [all?] the consumers of the emitters).
Updated•22 years ago
|
Status: NEW → ASSIGNED
Target Milestone: --- → mozilla1.3alpha
Comment 39•22 years ago
|
||
I am taking care of this issue...
Comment 40•22 years ago
|
||
I cnahe my mind, emitter should own the timer. Mime has nothing to do with the internal processing of the emitter, it just feed him with data. I'll post a patch right now...
Comment 41•22 years ago
|
||
this fix is based on previous patch, this time it use a timer. I havn't tested this patch on slow connection (I don't have one :-( ) but on broadband even with large image (400K), I don't see a difference! same with previous patch. Mike, do you have couple free cycles to test this patch? would be nice. Thanks
Attachment #101985 -
Attachment is obsolete: true
Comment 42•22 years ago
|
||
Good news and bad news. The good news is that the patch technically works in the sense that OnWrites are coalesced into intermittant OnFulls on the timer. The bad news is that this doesn't actually fix the problem. A few days ago Something on the trunk changed (I don't recall fiddling with any prefs in that period) that makes nsMimeBaseEmitter only very rarely see any OnWrites, even though data is coming down the wire and the minutes are ticking away. A small flurry of OnWrites happen when the stream starts loading, a giant armful of OnWrites happen when the stream has finished loading, but nothing happens in-between for the timer to be able to act upon. I first noticed this behaviour on the morning of 2002-11-12, or perhaps the day before, when I was wondering why my older simple patch wasn't doing the trick any more. It may have been happening a little longer though.
Comment 43•22 years ago
|
||
I had a version which instead of setting mFlushPending = PR_TRUE in OnWrite, I was doing it in the Write function. That did not help either!
Comment 44•22 years ago
|
||
move to futur until we get a new solution...
Severity: normal → enhancement
Target Milestone: mozilla1.3alpha → Future
Comment 45•22 years ago
|
||
*** Bug 182977 has been marked as a duplicate of this bug. ***
Comment 46•22 years ago
|
||
Related to comment #35: nsMimeBaseEmitter::OnWrite Function may receive a "granularity" parameter that would override the default - call OnWrite event after every single line is decoded, instead : - if num. lines is known, call the above event every X lines, where X is an integer fraction of the total number - if num lines is not known, call the event every X lines, where X is a predefined constant to give a reasonable granularity, but not create too much overhead. Considering the above, if a 200k jpeg will display progressively (while loading) in 25 steps... it ok, much better then after entire dl.
Comment 47•22 years ago
|
||
I hope nobody confuses progressive render of an image (general case, while loading) with progressive render of an image that has been encoded for progressive display. The very first time when I read the summary of this bug, I though it referred to a problem displaying progressive JPEGS in the news reader window, as opposed to normally encoded JPEGS... (png, etc)
Comment 48•22 years ago
|
||
*** Bug 185062 has been marked as a duplicate of this bug. ***
Comment 49•22 years ago
|
||
The main reason why I still use Netscape 3 for newsreading is the fact that images download progressively. Until Mozilla adopts this feature I will stick to Netscape.
Comment 50•21 years ago
|
||
Awww this bug has died... :-(
Comment 51•20 years ago
|
||
*** Bug 182983 has been marked as a duplicate of this bug. ***
Updated•20 years ago
|
Product: MailNews → Core
Comment 52•16 years ago
|
||
Ducarroz, are you still working on these? If not, please reassign to default assignee.
Comment 53•16 years ago
|
||
According to wsmwk, ducarroz is no longer taking bugs any more.
Assignee: ducarroz → nobody
Status: ASSIGNED → NEW
QA Contact: tpreston → networking.news
Assignee | ||
Updated•15 years ago
|
Product: Core → MailNews Core
Updated•15 years ago
|
Attachment #106061 -
Attachment is obsolete: true
Comment 54•15 years ago
|
||
Comment on attachment 106061 [details] [diff] [review] Proposed fix, v2 Patch has bitrotted: $ patch -p1 --dry-run < ~/Desktop/patch.txt (Stripping trailing CRs from patch.) patching file nsMimeBaseEmitter.cpp Hunk #1 FAILED at 136. Hunk #2 FAILED at 360. Hunk #3 FAILED at 378. Hunk #4 FAILED at 990. Hunk #5 succeeded at 995 (offset -44 lines). 4 out of 5 hunks FAILED -- saving rejects to file nsMimeBaseEmitter.cpp.rej (Stripping trailing CRs from patch.) patching file nsMimeBaseEmitter.h Hunk #1 succeeded at 54 (offset -4 lines). Hunk #2 FAILED at 84. Hunk #3 FAILED at 98. Hunk #4 succeeded at 172 (offset -6 lines). 2 out of 4 hunks FAILED -- saving rejects to file nsMimeBaseEmitter.h.rej
Updated•14 years ago
|
Component: Networking: NNTP → MIME
QA Contact: networking.nntp → mime
Updated•2 years ago
|
Severity: normal → S3
You need to log in
before you can comment on or make changes to this bug.
Description
•