When an ImageDocument is loading, it creates an <img> tag, which triggers a beforeload event. However, the event corresponds to the main resource, and can cause a crash if the event is handled and preventDefault() is called. <rdar://problem/11309013>
Created attachment 142094 [details] Patch
Committed r117185: <http://trac.webkit.org/changeset/117185>
Why is it safe to dispatch beforeload on image documents? This appears to be as dangerous as elsewhere, because a handler can do evil things.
(In reply to comment #3) > Why is it safe to dispatch beforeload on image documents? This appears to be as dangerous as elsewhere, because a handler can do evil things. I'm a little confused about your question. I don't think anyone here said it is safe to dispatch beforeload on image documents. But the bug here was really that we were dispatching beforeLoad for an <img> element inside the image document... but in reality the resource was already being loaded; It was the main resource for the document. Logically - since we don't do beforeLoad for main resources - the resource for an image document shouldn't be any different, and especially not by representing it as an <img> element which is an implementation detail of image documents.
I'm not entirely sure why this happens, but by calling dispatchPendingBeforeLoadEvent() in that branch actually prevents the beforeload event from reaching any handlers. It also triggers logic essential to making images show up in the first place. If the second branch is taken, it triggers the beforeload event after handlers get installed, thus the source of the bug. Skipping both branches misses the logic essential to making the images show up. From an objective standpoint, this means that some code is in the wrong place and needs to be refactored. However, I've been unsuccessful at finding where the aforementioned logic takes place, as it's nowhere obvious. I've filed bug 86658 as a followup.
(In reply to comment #5) > I'm not entirely sure why this happens, but by calling dispatchPendingBeforeLoadEvent() in that branch actually prevents the beforeload event from reaching any handlers. It also triggers logic essential to making images show up in the first place. If the second branch is taken, it triggers the beforeload event after handlers get installed, thus the source of the bug. Skipping both branches misses the logic essential to making the images show up. From an objective standpoint, this means that some code is in the wrong place and needs to be refactored. However, I've been unsuccessful at finding where the aforementioned logic takes place, as it's nowhere obvious. > > I've filed bug 86658 as a followup. I assumed that the very first check in that method - if (!m_hasPendingBeforeLoadEvent) - failed and it bailed out. But if calling that method actually does other stuff relevant to the image showing up, then my assumption can't possibly have been the case. I strongly urge you to explore this soon. Now both Alexey and I are confused.
> I'm a little confused about your question. I don't think anyone here said it is safe to dispatch beforeload on image documents. That's what the new code says, in a very direct manner. Per Jeffrey's answer, the code lies about what it does, however we don't appear to have complete understanding. > I'm not entirely sure why this happens, but by calling dispatchPendingBeforeLoadEvent() in that branch actually prevents the beforeload event from reaching any handlers. Perhaps it fires the event earlier, so handlers installed on img element don't have a chance to run? But handlers installed on ancestors would fire, and in a manner that's even worse than before?
(In reply to comment #7) > > I'm not entirely sure why this happens, but by calling dispatchPendingBeforeLoadEvent() in that branch actually prevents the beforeload event from reaching any handlers. > > Perhaps it fires the event earlier, so handlers installed on img element don't have a chance to run? But handlers installed on ancestors would fire, and in a manner that's even worse than before? The event doesn't bubble, so ancestors never see it. I've stepped through the code somewhat thoroughly and it looks like it bails out without actually doing anything noteworthy, but if I skip that code entirely, then img tags don't work. So it looks like some constructor somewhere must be doing the relevant work, I'm just not sure which one.
> representing it as an <img> element which is an implementation detail of image documents I've been under the impression that HTML5 specifies this, however I cannot find any proof now.
> The event doesn't bubble, so ancestors never see it. That's not what I'm seeing in my testing - the below snippet logs both "bl" and "bl2". <body> <script> document.body.addEventListener("beforeload", function() { console.log('bl') }, true); document.addEventListener("beforeload", function() { console.log('bl2') }, true); </script> <img src="aaa"> <img src="bbb">
Well, that's a capturing listener, so it doesn't actually contradict what you said about bubbling, but it still shows that listeners are invoked on ancestors.
(In reply to comment #11) > Well, that's a capturing listener, so it doesn't actually contradict what you said about bubbling, but it still shows that listeners are invoked on ancestors. In the capture phase, events are seen by all ancestors. In the bubbling phase, they're only seen if they bubble. (In reply to comment #9) > > representing it as an <img> element which is an implementation detail of image documents > > I've been under the impression that HTML5 specifies this, however I cannot find any proof now. See the navigation section.