1173716 - alert() spins the event loop, which can cause other alert() calls to happen and cover up the older alert

Status: UNCONFIRMED

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

Description

[tcaudilllg]

User Agent: Mozilla/5.0 (Windows NT 5.1; rv:38.0) Gecko/20100101 Firefox/38.0
Build ID: 20150525141253

Steps to reproduce:

imageTest = new Image();
imageTest.onload = checkout;
imageTest.src = "whatever.png";
alert("foo");


function checkout () {

    alert("bar");
}



Actual results:

alert("bar") happens before alert("foo");


Expected results:

May only be happening with local files, but it works in Chrome. Used to work in Firefox, as well.

Comment 1

[tcaudilllg]

I expect this to be fixed by the end of the day. Compliance with DOM level 1 is the least that should be expected of the institution that has screamed the loudest about standards compliance.

Comment 2

[Boris Zbarsky [:bzbarsky]]

> alert("bar") happens before alert("foo");

No, what happens is that alert("foo") happens first.  This spins the event loop, which causes the image load to complete, the image load handler to fire, and the second alert() call to happen.  The second alert is then shown on top of the first alert.  Since the image loads quickly, you only perceive the state with the second alert on top, click it away, and see the (older) first alert.  If the image loaded slowly enough you'd see the full sequence of events here.

You should try console.log() instead of alert if you want to see the order those two code positions are actually reached in without the nested event loop interference.

Note that DOM level 1 doesn't define the behavior of alert() at all (or of the image load event, for that matter), so there is no DOM level 1 compliance issue here.

Comment 3

[tcaudilllg]

Alright, level 2 then.

I know the event is misfiring because I'm using a counter to track img object loads before drawing to canvas. The counter is jumping to the max and rendering before load, causing drawImage to fail. This is not not happening in Chrome (and it didn't used to in FF before 38).

Comment 4

[Boris Zbarsky [:bzbarsky]]

> Alright, level 2 then.

alert() isn't defined by DOM level 2 either.  Or level 3, for that matter.

> I know the event is misfiring because I'm using a counter to track img object loads
> before drawing to canvas.

Please feel free to post a testcase or link to testcase that shows the actual issue you're encontering.  I'll be happy to take a look at it.

Comment 5

[tcaudilllg]

https://jsfiddle.net/jdctgsp1/

It makes a game map based on the data in a file.
You'll have to plug in values for width and height and cell size (make width/height kinda small, like 20 or 30), and give it a file to analyze. You'll need to give it a list of images to draw,
separated by line breaks, in the box below the Draw Map button. Click Draw Map to use.

Comment 6

[tcaudilllg]

(In reply to tcaudilllg from comment #5)
> https://jsfiddle.net/jdctgsp1/
> 
> It makes a game map based on the data in a file.
> You'll have to plug in values for width and height and cell size (make
> width/height kinda small, like 20 or 30), and give it a file to analyze.
> You'll need to give it a list of images to draw,
> separated by line breaks, in the box below the Draw Map button. Click Draw
> Map to use.

One more thing: you have to put a starting offset for read in the box after "Draw Map from".

Comment 7

[Boris Zbarsky [:bzbarsky]]

> and give it a file to analyze

What sort of file?

In case it wasn't clear, what would be ideal here is a testcase with clear steps to reproduce that can definitely be followed by anyone without screwing them up.  Comment 5 is not it.  :(

Comment 8

[Boris Zbarsky [:bzbarsky]]

Oh, and the other thing that's needed is a clear description of the expected behavior and the actual behavior you see, so I can tell whether I'm reproducing the problem you're seeing.

Comment 10

[brodhead.tom]

@Kyle Huey: This is not a duplicate issue. The alert() has nothing to do with the problem. Load the test case, examine what's happening internally, and see for yourself.

Comment 11

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

I did.  If you replace the alert with console.log the number show up in the console in the expected (increasing) order.

Comment 12

[brodhead.tom]

loaded_counter is initialized prematurely, before the images are fully loaded:

<!DOCTYPE html>
<img onload="loadImage()" src="image01.jpg" /></a>
<img onload="loadImage()" src="image02.jpg" /></a>
<img onload="loadImage()" src="image03.jpg" /></a>

<script>
var loaded_counter = 0;
function loadImage() {
	loaded_counter++;
	alert(loaded_counter);
}
</script>

Comment 13

[brodhead.tom]

Strip out the errant </a> tags above...my mistake. The example still proves the point. Run this in Firefox and then in Chrome and notice the difference in behavior.

Comment 14

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Again, I did that.  Replace the alert with a console.log and run the testcase ...

Comment 15

[brodhead.tom]

Okay, here is the proof, using your methodology. Select 3 *LARGE* image files--mine were 26MB each--and run the following code with the console open. You'll see that the console quickly logs 1 2 3 for the three counter variables...but the images aren't fully loaded yet! Using small image files won't reveal this problem, because they finish loading asynchronously before or at the same time that the counter variables are printed to the console log. You need to use HUGE images, like 25MB or greater, and then you'll see that onload() is being executed before the loading process is complete.

This was causing my code to fail on Firefox, but not on Chrome, Safari, and even IE. I had to devise a special routine to account for Firefox's failings here in order for my site to function correctly.

Here, try this code and please let me know if it doesn't provide the same result that I got:

<!DOCTYPE html>
<?php
$time1 = time();
$time2 =+ $time1;
$time3 =+ $time2;
?>
<img onload="loadImage()" src="image01.jpg?t=<?php echo $time1;?>" />
<img onload="loadImage()" src="image02.jpg?t=<?php echo $time2;?>" />
<img onload="loadImage()" src="image03.jpg?t=<?php echo $time3;?>" />

<script>
var loaded_counter = 0;
function loadImage() {
	loaded_counter++;
	console.log(loaded_counter);
}
</script>

Comment 16

[Boris Zbarsky [:bzbarsky]]

If your issue is not specific to alert(), please take it back to bug 1211528.

Comment 18

[Jonathan Watt [:jwatt]]

Mass bug change to replace various 'parity' whiteboard flags with the new canonical keywords. (See bug 1443764 comment 13.)

Comment 19

[Richard Cheng]

Hello, can I try to solve this bug?
also I want to ask which file should I look to reproduce and solve this bug?

Comment 20

[Olli Pettay [:smaug][[e-mail]]]

This is a really complicated issue to fix.
Alert is implemented here https://searchfox.org/mozilla-central/rev/87ba454e5c68ff77dff9acb9d7b0b51d6df12d11/dom/base/nsGlobalWindowInner.cpp#3870-3880 but the issue is more about spinning event loop.
We should somehow block running tasks which might trigger any JS on the web page to run while still spinning the event loop so that underlying privileged code (native C++ and JS) can still run.

This is definitely not a good first bug ;)

https://bugzilla.mozilla.org/buglist.cgi?keywords_type=allwords&query_format=advanced&classification=Client%20Software&classification=Developer%20Infrastructure&classification=Components&classification=Server%20Software&classification=Other&list_id=16580470&keywords=good-first-bug%2C%20&resolution=--- has a list of possibly easier bugs.

Comment 21

[BugBot [:suhaib / :marco/ :calixte]]

Redirect a needinfo that is pending on an inactive user to the triage owner.
:edgar, since the bug has recent activity, could you have a look please?

For more information, please visit BugBot documentation.

Comment 22

[Edgar Chen [:edgar] (Parental leave until Oct. 09.)]

I think comment #20 already provides enough information.