Bug 86658 – ImageLoader can still dispatch beforeload events for ImageDocuments

• Status: RESOLVED
• Resolution: FIXED
• Component: Page Loading

[Vicki Pfau]

When loading images, some of the logic regarding making the images appear on the page seems to occur somewhere within the beforeload logic (somewhere within code called by ImageLoader::dispatchPendingBeforeLoadEvent()). This means that a beforeload event needs to be dispatched on all loaded images, even within some contexts like ImageDocuments.

While calling dispatchPendingBeforeLoadEvent while the image is being first updated (ImageLoader::updateFromElement), instead of being deferred (as happens when the page has a beforeload handler) ensures that listeners never actually sees the event, it still shouldn't fire the event in the first place. This means that the logic for connecting the images so they appear needs to be moved out of whatever dispatchPendingBeforeLoadEvent calls and into a separate function.

I've tried to find where this logic occurs, but I have as of yet been unsuccessful.

<rdar://problem/11465863>

[Vicki Pfau]

Created attachment 142322 [details]
Patch

[Brady Eidson]

Comment on attachment 142322 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=142322&action=review

> Source/WebCore/loader/ImageLoader.cpp:198
> -        m_hasPendingBeforeLoadEvent = newImage;
> +        if (!m_element->document()->isImageDocument())
> +            m_hasPendingBeforeLoadEvent = newImage;

I would consider forcing m_hasPendingBeforeLoadEvent to false for image documents, and make sure whatever other bits of related data are set appropriately (if any)

[Vicki Pfau]

(In reply to comment #2)
> (From update of attachment 142322 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=142322&action=review
> 
> > Source/WebCore/loader/ImageLoader.cpp:198
> > -        m_hasPendingBeforeLoadEvent = newImage;
> > +        if (!m_element->document()->isImageDocument())
> > +            m_hasPendingBeforeLoadEvent = newImage;
> 
> I would consider forcing m_hasPendingBeforeLoadEvent to false for image documents, and make sure whatever other bits of related data are set appropriately (if any)

I'm not sure that's necessary, but I suppose it is nice to be explicit. The only associated bit seems to be regarding queuing of events in the event sender--which we don't do in this case anyway, so that's fine. I'll upload a version with the flag set explicitly.

[Vicki Pfau]

Created attachment 142332 [details]
Patch

[Brady Eidson]

Comment on attachment 142332 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=142332&action=review

> Source/WebCore/ChangeLog:12
> +
> +        Prevent flags regarding sending beforeload events from being set on ImageDocuments.
> +
> +        No new tests.
> +

Might want to reiterate the same reason why not from the last change.

[Vicki Pfau]

Committed r117336: <http://trac.webkit.org/changeset/117336>