309141 - [FIX]Smileys in Chatzilla embedded via content: url[...] are not displayed the first time

Status: RESOLVED FIXED

(Core :: CSS Parsing and Computation, defect, P1)

Description

[Frank Wein [:mcsmurf]]

In a current nightly trunk build smileys are sometimes not displayed in
Chatzilla, this /could/ be a fall-out from Bug 11011. To reproduce just join any
channel and type any smileys like :), :D, :/, :DD, XD, etc. and repeat those
smilies. It seems the first time you use a smiley in Chatzilla it is not
displayed, the second, third, etc. time you use it it works. DOMI shows the XML
for the smileys is inserted correctly (and if you hover over where the smiley
should be, you get the title attribute as a tooltip as it should be). The CSS
for inserting the smilies can be found under
http://lxr.mozilla.org/mozilla/source/extensions/irc/xul/content/output-base.css#355.

Comment 1

[Frank Wein [:mcsmurf]]

The XML looks like this for inserting a smiley (from DOMI):
<span>
  <span class="chatzilla-emote-txt" type="face-laugh">
:D  </span>
  <span class="chatzilla-emote" type="face-laugh" title=":D"/>
</span>

Comment 2

[Boris Zbarsky [:bzbarsky]]

Fun times.  The only reason the second and later smilies show at all is because
of that sync mess in imagelib, which causes an UpdateState call from
OnStopDecode.  And of course I never managed to catch it in testing since I'd
only used images in generated content that I'd already loaded once before that
browsing session.  The basic problem is that when we set up the generated image
content we don't update its state to indicate that it now has a request....

Comment 3

[Boris Zbarsky [:bzbarsky]]

Attachment: Patch

Comment 4

[Christian :Biesinger (don't email me, ping me on IRC)]

so GenConImageContent handles the load behind nsImageLoadingContent's back? how
about instead providing a protected method "CloneRequest(imgIRequest*)" which
handles this? that could be on the IDL too.

Comment 5

[Boris Zbarsky [:bzbarsky]]

If you think that would be a better idea, sure.  I can do that...

Comment 6

[Christian :Biesinger (don't email me, ping me on IRC)]

yes, I'd prefer that.

Comment 7

[Boris Zbarsky [:bzbarsky]]

Attachment: With protected method

I'd rather not put this on the IDL until I've had time to think about that...

Comment 8

[Christian :Biesinger (don't email me, ping me on IRC)]

Comment on attachment 196670 [details] [diff] [review]
With protected method

r=biesi, but why "will try to notify" instead of "will notify"?

Comment 9

[Boris Zbarsky [:bzbarsky]]

Because it may not notify if there is no current doc.

Comment 10

[Christian :Biesinger (don't email me, ping me on IRC)]

but the same is true for ImageURIChanged, right? that one doesn't say "try"

Comment 11

[Boris Zbarsky [:bzbarsky]]

Fair enough.  I'll make sure the two say the same thing before checking in.

Comment 12

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 196670 [details] [diff] [review]
With protected method

Isn't some of the cancelling going on in this function cancelling stuff that
should never be going on?  If so, should some of this be assertions rather than
actually doing things?	Either way, sr=dbaron, since I trust you guys and I
don't think it's worth my remembering how this code actually works.

Comment 13

[Boris Zbarsky [:bzbarsky]]

I could make them assertions as things stand, yeah.  I figured I might as well
make the method generally useful, though.

Comment 14

[Boris Zbarsky [:bzbarsky]]

Fixed.