Closed Bug 583825 Opened 11 years ago Closed 11 years ago

Progressive decoding/display of images no longer works

Categories

(Core :: ImageLib, defect)

defect
Not set
normal

Tracking

()

VERIFIED FIXED
mozilla2.0b4
Tracking Status
blocking2.0 --- beta4+

People

(Reporter: joe, Assigned: bholley)

References

()

Details

(Keywords: regression)

Attachments

(1 file)

Visit the URL mentioned. Firefox loads the image properly, but doesn't progressively decode it - instead, you get the alt text while the image downloads, and then once it's done downloading/decoding, you get the full image displayed.
blocking2.0: --- → beta4+
this is bad.
Setting image.mem.decodeondraw to true seems to fix this (or at least get around it) for me.
Regression window:

Works:
http://hg.mozilla.org/mozilla-central/rev/5f857be14db9
Mozilla/5.0 (Windows; Windows NT 6.1; WOW64; rv:2.0b3pre) Gecko/20100727
Minefield/4.0b3pre ID:20100728145944

Fails:
http://hg.mozilla.org/mozilla-central/rev/b8b62b351c09
Mozilla/5.0 (Windows; Windows NT 6.1; WOW64; rv:2.0b3pre) Gecko/20100728
Minefield/4.0b3pre ID:20100728152620

Pushlog:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=5f857be14db9&tochange=b8b62b351c09
Duplicate of this bug: 584144
Oh hooray, a regression from my bug!
OS: Mac OS X → All
Hardware: x86 → x86_64
This is not an x86_64-only bug.  I see this also on XP 32-bit.
Summary: progessive decoding/display no longer works → progressive decoding/display no longer works
Hardware: x86_64 → All
Keywords: regression
Summary: progressive decoding/display no longer works → Progressive decoding/display of images no longer works
Figured this bug out.

The issue has to do with the tricky stuff we do with OnStartContainer, as discussed in bug 572520 comment 31. Before that patch, we were setting stateHasSize after sending OnStartContainer, but we'd send imgIRequest::STATUS_SIZE_AVAILABLE _before_. In bug 572520, this was all rolled together into RecordStartContainer(), so they both got set afterwards.

This is a problem, because consumers of OnStartContainer expect STATUS_SIZE_AVAILABLE to be set. In particular, nsImageLoadingContent calls UpdateImageState(), which doesn't set mLoading=false because it doesn't find STATUS_SIZE_AVAILABLE (note the abuse of the concept of "loading" here - in nsImageLoadingContent, "loading" means "size not yet available"). This means that nsImageFrame::ShouldCreateImageFrameFor() returns falls during reflow, which means that the frame doesn't get created until we do another reflow when we get the whole image.

Patch coming right up.
Status: NEW → ASSIGNED
Attached patch patch v1Splinter Review
Since really we want to send OnStartContainer once per-proxy, it doesn't make the most sense to have such handling be done in imgRequest and imgStatusTracker. This patch moves all that into imgRequestProxy, and fixes the bug.
Attachment #464503 - Flags: review?(joe)
Comment on attachment 464503 [details] [diff] [review]
patch v1

r+, looks great. File a followup for the very-likely-to-be-intermittently-faily test for this functionality.
Attachment #464503 - Flags: review?(joe) → review+
(In reply to comment #9)
> Comment on attachment 464503 [details] [diff] [review]
> patch v1
> 
> r+, looks great. File a followup for the very-likely-to-be-intermittently-faily
> test for this functionality.

Filed bug 586078.
Is there any chances of seeing this merged to trunk any time soon, or will it have to wait(forever) for that test?
(In reply to comment #11)
> Is there any chances of seeing this merged to trunk any time soon, or will it
> have to wait(forever) for that test?

No, this is ready to land. It's just difficult to land patches right now because the tree is so busy.
This is marked as a beta4 blocker, so it needs to land by Friday.  Please ensure that it does so (by landing it yourself, or getting checkin-needed on the bug while keeping in mind the points raised in http://blog.bonardo.net/2010/06/22/so-youre-about-to-use-checkin-needed)
This just landed:
http://hg.mozilla.org/mozilla-central/rev/706f72b0c151

Resolving fixed.
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Verified fixed with Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:2.0b4pre) Gecko/20100813 Minefield/4.0b4pre
Status: RESOLVED → VERIFIED
Target Milestone: --- → mozilla2.0b4
Duplicate of this bug: 587613
You need to log in before you can comment on or make changes to this bug.