Closed
Bug 901146
Opened 11 years ago
Closed 11 years ago
Fix misleading comment in nsImageFrame.cpp
Categories
(Core :: Layout: Images, Video, and HTML Frames, defect)
Core
Layout: Images, Video, and HTML Frames
Tracking
()
RESOLVED
FIXED
mozilla31
People
(Reporter: seth, Assigned: seth)
Details
Attachments
(1 file, 1 obsolete file)
(deleted),
patch
|
tnikkel
:
review+
|
Details | Diff | Splinter Review |
(From bug 825771 comment 36:)
The current behavior doesn't make sense because BuildDisplayList will display the image if the first frame is partially decoded, but if it wasn't available when BuildDisplayList was called so that we end up calling DisplayAltFeedback when painting instead, then DisplayAltFeedback discovers that the first frame is now partially decoded, it won't actually paint it unless the first frame is completely decoded. Whether the image gets displayed before the first frame is fully decoded is therefore determined by a race on the timing of the call to BuildDisplayList.
Assignee | ||
Comment 1•11 years ago
|
||
Proposed patch. This is a one-liner that makes us display the image in DisplayAltFeedback if we have STATUS_SIZE_AVAILABLE, rather than waiting for STATUS_FRAME_COMPLETE. This matches the BuildDisplayList code.
Try job here: https://tbpl.mozilla.org/?tree=Try&rev=b3f5beb781a7
Attachment #785276 -
Flags: review?(dholbert)
Comment 2•11 years ago
|
||
The only caller of DisplayAltFeedback is here http://mxr.mozilla.org/mozilla-central/source/layout/generic/nsImageFrame.cpp#1064 which shows that aRequest here is either gIconLoad->mLoadingImage or gIconLoad->mBrokenImage, not the request of the image we are asked to display for this image frame. So it's a different request from the request in BuildDisplayList.
Assignee | ||
Comment 3•11 years ago
|
||
Wow, good catch. Alright, let's update the comment at the top of that block to clarify what the situation is.
This also makes it clear that I don't need to touch that code at all in bug 825771, so I'll remove that portion of the patch on that bug. (And request review from you, just to make sure I don't let any other mistakes slip through!)
Attachment #785319 -
Flags: review?(tnikkel)
Assignee | ||
Updated•11 years ago
|
Attachment #785276 -
Flags: review?(dholbert)
Assignee | ||
Updated•11 years ago
|
Attachment #785276 -
Attachment is obsolete: true
Assignee | ||
Updated•11 years ago
|
Summary: nsImageFrame should display the image in the same cases in DisplayAltFeedback and BuildDisplayList → Fix misleading comment in nsImageFrame.cpp
Updated•11 years ago
|
Attachment #785319 -
Flags: review?(tnikkel) → review+
Assignee | ||
Comment 4•11 years ago
|
||
Wow, this somehow got removed from my patch queue without getting pushed. Nine months ago.
Pushed:
https://hg.mozilla.org/integration/mozilla-inbound/rev/d71e43379733
Comment 5•11 years ago
|
||
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla31
Updated•6 years ago
|
Product: Core → Core Graveyard
Updated•6 years ago
|
Product: Core Graveyard → Core
You need to log in
before you can comment on or make changes to this bug.
Description
•