Closed
Bug 853628
Opened 11 years ago
Closed 11 years ago
Bookmarks/History favicons don't display after bug 716140
Categories
(Core :: Widget: Cocoa, defect)
Tracking
()
RESOLVED
FIXED
mozilla22
People
(Reporter: joe, Assigned: joe)
References
Details
Attachments
(1 file)
785 bytes,
patch
|
smichaud
:
review+
|
Details | Diff | Splinter Review |
nsMenuItemIconX makes some assumptions about the order events are delivered in that aren't true any more. This patch fixes them.
Attachment #727878 -
Flags: review?(smichaud)
Comment 1•11 years ago
|
||
Comment on attachment 727878 [details] [diff] [review] don't wait for load to finish, wait for decode What's the specific problem that this patch fixes? Is it that LOAD_COMPLETE now happens "too early", so that mIconRequest can get cancelled "too early"? Would it *always* have made better sense to wait for DECODE_COMPLETE?
Assignee | ||
Comment 2•11 years ago
|
||
(In reply to Steven Michaud from comment #1) > What's the specific problem that this patch fixes? In tonight's respun nightly, the History menu always shows blank icons the first time you open it; the second time, they draw properly. > Is it that LOAD_COMPLETE now happens "too early", so that mIconRequest can > get cancelled "too early"? Right. > Would it *always* have made better sense to wait for DECODE_COMPLETE? Yes.
(In reply to Joe Drew (:JOEDREW! \o/) from comment #0) > [->Insert Function<-] makes some assumptions about the order events are delivered > in that aren't true any more. This patch fixes them. Could something similar to this (i.e. waiting for load but not decode to complete) be at the heart of bug 853337? I haven't yet reproduced but just wondering.
Comment 4•11 years ago
|
||
Comment on attachment 727878 [details] [diff] [review] don't wait for load to finish, wait for decode >> Would it *always* have made better sense to wait for DECODE_COMPLETE? > > Yes. That's good enough for me :-)
Attachment #727878 -
Flags: review?(smichaud) → review+
Assignee | ||
Comment 5•11 years ago
|
||
(In reply to IU from comment #3) > (In reply to Joe Drew (:JOEDREW! \o/) from comment #0) > > [->Insert Function<-] makes some assumptions about the order events are delivered > > in that aren't true any more. This patch fixes them. > > Could something similar to this (i.e. waiting for load but not decode to > complete) be at the heart of bug 853337? I haven't yet reproduced but just > wondering. That's what I suspect is going on for the favicons, but right now I'm just guessing.
Assignee | ||
Comment 6•11 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/69f965c0fd46
Target Milestone: --- → mozilla22
Assignee | ||
Comment 7•11 years ago
|
||
Backed out since it was part of a push that mysteriously crashed in crashtests. Pushed it on its own: https://tbpl.mozilla.org/?tree=Try&rev=4cd58fb0b7f0
Comment 8•11 years ago
|
||
Not you, it happened again above your backout.
Comment 9•11 years ago
|
||
Relanded in https://hg.mozilla.org/integration/mozilla-inbound/rev/4e743f7bf114
Comment 10•11 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/4e743f7bf114
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Comment 11•11 years ago
|
||
Patch didn't work for me Joe. I have to disable azure to get my favicons to always display.
Comment 12•11 years ago
|
||
Just noticed it says platform Mac and I'm on Windows 8. Is there a patch for Windows?
You need to log in
before you can comment on or make changes to this bug.
Description
•