309076 - Image ALT text shows instead of image until mouseover when Adblock is installed

Status: RESOLVED WONTFIX

(Core :: DOM: Core & HTML, defect)

Description

[Ryan VanderMeulen [:RyanVM]]

This is most likely a regression from the checkin on bug 11011.

Images with alt tags have the text shown instead of the actual image. If you
mouseover the text, it switches to the image instead. I've found the most
reliable way to reproduce the problem is to reload the page. The images tend to
show upon the first rendering of it and alt text on subsequent reloads.

The URL given above is a site run by myself. If I open that exact same page
locally, I can't reproduce the error.

I've also seen this issue when replying to a message on a vBulletin3 forum, so
it's not something isolated to my website.

Comment 1

[Ryan VanderMeulen [:RyanVM]]

Attachment: Screenshot #1 of problem

The top of the image show the image on the first loading of the page. The
bottom shows what happens after it's reloaded.

Comment 2

[Ryan VanderMeulen [:RyanVM]]

Attachment: Screenshot #2 of problem

The top of the image show the image on the first loading of the page. The
bottom shows what happens after it's reloaded.

Comment 3

[Boris Zbarsky [:bzbarsky]]

I can't seem to reproduce this, but my build has the Tp regression fix attempts
in it, and the second one could very well help here...

Comment 4

[Ryan VanderMeulen [:RyanVM]]

Still reproducing even with Tp patch #2 included in the build

Comment 5

[Boris Zbarsky [:bzbarsky]]

The problem here is that under BindToTree we call ImageURIChanged, which calls
ShouldLoad().  The adblock ShouldLoad() impl accesses document.defaultView on an
"unsafe" document, which calls ResolveName() on the doc, which flushes content
notifications and hence frame construction.

Hence we get a frame constructed while halfway through ImageURIChanged().  Since
we're coming from BindToTree, aNotify is false, so we don't notify at the end of
ImageURIChanged().

Frankly, I think the right fix here is for adblock to use an XPCNativeWrapper
for the document.  That would prevent the ResolveName call and would be a better
idea in general.

I suppose could add some hacks to nsImageLoadingContent to catch this case and
deal, but they would all cause performance degradation in various cases, so I
would really rather not.

Comment 6

[Boris Zbarsky [:bzbarsky]]

So to make this clear, I'm suggesting we WONTFIX this bug and get adblock fixed.
 Any objections?

Comment 7

[Caleb]

(In reply to comment #6)
> So to make this clear, I'm suggesting we WONTFIX this bug and get adblock fixed.
>  Any objections?

No. :)

Fix Extension > Add hacks to code that cause perf degradation

Comment 8

[Boris Zbarsky [:bzbarsky]]

Quite frankly, I was asking for the opinion of the layout and DOM peers I cced
on the bug when I asked that question.

Comment 9

[Will May]

*** Bug 309465 has been marked as a duplicate of this bug. ***

Comment 10

[Dave Townsend [:mossop]]

*** Bug 311062 has been marked as a duplicate of this bug. ***

Comment 11

[Matt]

What the hell does Adblock have to do with this?

Comment 12

[Boris Zbarsky [:bzbarsky]]

Thanks for asking so politely, but you should really try reading the bug.  It
was explained in quite some detail in comment 5.

Comment 13

[Michael]

Matt, I've been maintaining a fixed version of Adblock here:
http://aasted.org/adblock/viewtopic.php?t=2264

Boris is there a better way of accessing arrays that Adblock creates on nodes
extensively without using wrappedJSObject?

Comment 14

[Boris Zbarsky [:bzbarsky]]

Michael, not really.

Comment 15

[Matt]

Oh crud, I'm sorry, I didn't see that comment. >_<

I've notified the maintainer of Adblock Plus about the problem including a link
to the bug and the way to fix it.  Fixing the bug in Adblock Plus wouldn't cause
it to stop working with 1.0.x, would it?

Comment 16

[Will May]

Adblock Plus has already been updated (in the development version, at least).

http://p2.forumforfree.com/development-builds-vt5-adblockplus.html

Comment 17

[Tomas Herceg]

*** Bug 315280 has been marked as a duplicate of this bug. ***