According to the spec we are following for alt text processing: http://www.hixie.ch/specs/alttext ...we should be rendering the alt text inline for images that have alt text, have no height and width attributes, and have not yet loaded. Currently we don't show anything, which means if such an image is a link, the link is unclickable until we display the image. Which makes the link permanently unclickable if the remote site is having DNS issues or similar, since then we'll be waiting for ages before deciding the image is broken. I see this on my portal: http://☺.damowmow.com/ ...quite often (the comics at the bottom are often on dodgy servers).

So the problem with doing this is that it'd involve a reframe or _attempted_ reframe when we get OnStopContainer or something.... which means flushing out frame construction first. Which means a pageload time hit, possibly a pretty substantial one (the initial patch for bug 11011 made pageload times 15% higher for just such a reason). I suppose we could try to mitigate the impact by only doing the update batch if MayHaveFrame() is true in OnStopContainer. Not sure what the perf impact might be then; maybe worth testing. There's another issue here. Should we be showing the alt text till the image "has loaded" or till it "has started loading"? I would say the latter, where "started" means we've at least decoded the image format header and know the intrinsic image size. This way large images (eg digital photos) show up gradually, and progressive jpegs actually work. Not to mention that some image formats (JPEG push stuff comes to mind) never actually finish loading. David, biesi, what do you think?

well, I don't think you can avoid the reframe... I'd expect that in most cases a page will have more than two images from the same server, and necko only does 2 persistent connections per server... which means it's likely that you need the alt text frame, which means you have to do that frame reconstruction.

Hmm... That's true... I guess with the changes I want to make in bug 309986 at least the content state change itself won't flush the sink. The restyle will have to, but with any luck it'll be sorta batched with other things (if nothing else, the restyle won't happen till we go back out to the event loop). So it might not cause the sort of impact the initial bug 11011 patch did... In any case, I might try doing this after I land bug 309986 and see how Tp deals.

Why do we not require a reframe today? Are we inserting a 0x0 replaced inline frame (or whatever Gecko calls it) as we wait for image data? Is there no way to make replacing an inline frame with a replaced element frame more efficient? :-)

> Why do we not require a reframe today? Are we inserting a 0x0 replaced inline > frame (or whatever Gecko calls it) as we wait for image data? The layout object is an nsImageFrame while we wait for image data. It typically draws a little placeholder icon if it actually has to paint before the image's size is known (which is pretty rare, what with paint suppression; you can get this on long pages with lots of images like some specs, though). > Is there no way to make replacing an inline frame with a replaced element > frame more efficient? :-) The problem is not the replacement itself. The problem is that said replacement must happen when the DOM and frame model are in a consistent state. During most of the load the DOM and frame model are NOT in such a state, because frame construction works much faster in chunks; the bigger the chunks the smaller the total time taken. This needs to be balanced against incremental rendering, of course. So the frame model will typically lack behind the DOM in terms of being constructed, and we will every so often build whole pieces of it. If we need to reconstruct a frame, we first need to bring the frame model up to date, and only then mess with it. If this happens a lot, we're basically just constructing the frame model in smaller chunks, which is slower. This is where the performance hit really comes from. All that said, with bug 309986 fixed we at least won't flush out the content sink's notifications on the state change itself. We'll still do it when the restyle event fires, but that should hopefully batch together multiple images and such... More importantly, we already have built-in flushes when we reflow, so as long as the reframes happen at points where we'd normally have seen reflows anyway (that is, off events) we might be ok perf-wise.

Some testcases: Quirks, no size: data:text/html,<img src="http://landfill.mozilla.org/ryl/slowResponse3.cgi?time=20" alt="Real image in 20 seconds"> Quirks, size: data:text/html,<img src="http://landfill.mozilla.org/ryl/slowResponse3.cgi?time=20" alt="Real image in 20 seconds" width="100" height="200"> Standards, no size: data:text/html,<!DOCTYPE HTML PUBLIC "" []><img src="http://landfill.mozilla.org/ryl/slowResponse3.cgi?time=20" alt="Real image in 20 seconds"> Standards, size: data:text/html,<!DOCTYPE HTML PUBLIC "" []><img src="http://landfill.mozilla.org/ryl/slowResponse3.cgi?time=20" alt="Real image in 20 seconds" width="100" height="200">

This implements the following: 1) When deciding whether to show a sized box for an image, if it's still loading do so only if its size is "specified" (which actually means its size is set and not set to percentages) or if we would show a sized box if it were broken. 2) Once enough image data comes in that we know the image size, reframe. 3) Matching on ::-moz-loading for the state involved (that is, prior to us getting enough data from the server to know what the image size is). This seems to pass all four of the testcases fine; I'd have to try it on tinderbox to see the perf impact.

might make sense to make nsObjectLoadingContent return this new state for eType_Loading, even though nothing uses it yet.

Sure, I can do that.

(btw bz the simplest strict mode DOCTYPE is just "<!DOCTYPE HTML>".)

Comment on attachment 197508 [details] [diff] [review] Proposed patch r=biesi with that change >size is "specified" (which actually means its size is >set and not set to percentages) that doesn't match my reading of HaveFixedSize, btw.

Ah, indeed. I'll fix that comment.

I tried checking this in and backed it out again -- it caused a 3% regression in Tp overall and on some of our test pages it was a 50% regression. I'll try to find some time to look into why these last were so hard-hit.

OK, so I've found two problems: 1) One of the test pages has markup like: <font> lots of stuff including blocks <img> </font> so reframing the image was very expensive because of bug 311615. Enough so that pageload on that page went up about 50% and that made the overall Tp number go up. 2) The end of a view update batch (eg the sort that happens after the restyle event is processed) flushes out reflow if there are any pending invalidates on the viewmanager. This seems to be happening just as much without this patch, though, so perhaps it's not an issue. So once bug 311615 is fixed I'll try relanding this.

Checked in again. This time, the Tp hit was less than 0.5%, if any, which I think is ok...

Could this be what's breaking smileys on chatzilla? The first time a smiley is used, it either doesn't show up or blinks and disappears. The next time it's used, it shows up fine, probably because it's already in cache.

(In reply to comment #17) > Could this be what's breaking smileys on chatzilla? The first time a smiley is > used, it either doesn't show up or blinks and disappears. The next time it's > used, it shows up fine, probably because it's already in cache. That sounds like bug 309141.

Samuel, that's possible, yes. Is there a bug on it? I can't test right now because chatzilla is completely busted by the js component fastload, but once that's fixed I can look...

Samuel, this was in fact causing the smily problem. I've filed bug 313656 on it.

Fwiw, this is now causing a wide page with scrollbars while the page is loading at: http://www.lidl.de/de/home.nsf/pages/c.o.pc.index