setTimeout doesn't work after calling alert from evalInSandbox

RESOLVED FIXED in mozilla16

Status

()

Toolkit
General
--
critical
RESOLVED FIXED
6 years ago
5 years ago

People

(Reporter: Crossrider, Unassigned)

Tracking

({regression})

2.0 Branch
mozilla16
regression
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Reporter)

Description

6 years ago
User-Agent:       Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10_6_6; en-US) AppleWebKit/534.16 (KHTML, like Gecko) Chrome/10.0.648.204 Safari/534.16
Build Identifier: Firefox 4.0

We have a simple evalInSandbox code with alert and then immediately the setTimeout (or setInterval) function, which has a simple alert inside it too.

The first alert executes, but then the alert in the setTimeout itself does not.
If we run the setTimeout on its own without the alert it works:

WORKING CODE EXAMPLE:
----
setTimeout(function() { alert('timeout') },2000);
----

NOT WORKING EXAMPLE:
----
alert("start");
setTimeout(function() { alert('timeout') },2000);
-----

After it stops working, even if you run the code without the alert, the setTimeout alert will not run.
ONLY restarting Firefox make it work again, but only when there is no alert prior to the seTimeout (or setInterval)

Reproducible: Always

Steps to Reproduce:
1.create an xpi with the which runs the above code with evalInSandbox.
2.see that it does work without alert
3.add the alert -- see that timeout is not executing
4. try again without alert - this time the timeout its not working
5. restart firefox
6. see how it works again, but not with the alert prior to the setTimeout.

repeat.
Actual Results:  
setTimetout and setInterval not working after alert in evalInSandbox

Expected Results:  
setTimeout and setInterval should work even after alert in evalInSandbox

the new alert design is not cool and sometime simply hangs.. but that's a whole different issue..

Comment 1

6 years ago
Created attachment 525378 [details]
reproducible test case for alert-in-sandbox breaking settimeout

This bug seriously affects Greasemonkey.  It will also break code _in the page_.

See our reports:
https://github.com/greasemonkey/greasemonkey/issues/1318
http://groups.google.com/group/greasemonkey-users/t/aed1ecc36fa9e404

Real users are having real problems because of this.  I came up with a very similar test case as a greasemonkey script (which will execute inside a sandbox):
https://gist.github.com/910628


In addition, this attachment is a simple test case which just calls alert, and a setTimeout for a second alert, from within a sandbox.  The second never shows.  It works fine in Firefox 3.6, but fails in 4.0.

Here's the actual JS code involved:
---------------------------------
function alertSettimeoutExercise() {
  var contentWin = gBrowser.contentWindow;
  var sandbox = new Components.utils.Sandbox(contentWin);
  sandbox.__proto__ = contentWin;
  Components.utils.evalInSandbox(
      'alert("first alert; works");' +
      'setTimeout(function() { alert("second alert; fails"); }, 0)',
      sandbox);
}
---------------------------------
In case it's easier for you to see it here than to dissect the XPI I attached.


Moreover, try this:

Open any page.  Paste (e.g.) into the address bar:

javascript:setTimeout(function() { alert("alert via timeout"); }, 0);void(0)

And you'll see an alert.  Trigger the exercise of this bug and paste it again: it will not work.  Content setTimeout() calls are broken after the call to alert() inside a sandbox referencing that content.  This breakage of content lasts for the entire life of the tab; navigating to a new page and pasting that snippet will still fail to show the expected alert, once the alert-in-sandbox has broken it.

I don't know the right flags to set to mark this as high priority (and won't risk doing the wrong thing), but please consider it as such.

Comment 2

6 years ago
My god. This is a critical bug, worth releasing a FF Patch today!

Updated

6 years ago
Severity: major → critical
Component: General → Extension Compatibility
OS: Mac OS X → All
QA Contact: general → extension.compatibility
Hardware: x86 → All
Version: unspecified → 4.0 Branch

Updated

6 years ago
Assignee: nobody → general
Component: Extension Compatibility → JavaScript Engine
Product: Firefox → Core
QA Contact: extension.compatibility → general
Version: 4.0 Branch → 2.0 Branch

Comment 3

6 years ago
I tested this in the web console - whcih executes everything in a sandbox:

alert("start");
setTimeout(function() { alert('timeout') },2000);
Status: UNCONFIRMED → NEW
Ever confirmed: true

Comment 4

6 years ago
(In reply to comment #3)
> I tested this in the web console - whcih executes everything in a sandbox:
> 
> alert("start");
> setTimeout(function() { alert('timeout') },2000);

I also meant to write that it does indeed seem to be a problem

Comment 5

6 years ago
I can't see how this is possibly a JSeng issue.

Chances are, it's an issue with the Firefox-side alert code (which suppresses alerts while other alerts are live).

Justin, can you take a look?
Assignee: general → nobody
Component: JavaScript Engine → General
Product: Core → Firefox
QA Contact: general → general
Hmm, strange.

Testing from the web console, I ran do |setTimeout(function() { alert('timeout') },2000);| repeatedly, and it works each time. But as soon as I do just a plain alert() without the timeout, doing calls with the setTimeout do nothing. I don't even hit a breakpoint at the top of nsGlobalWindow::Alert().

Next would be to see if the timeout's actually firing at all. Oh, hmm, I also see some errors in the console saying the NS_ENSURE_ARG_POINTER failed in nsDOMWindowUtils::LeaveModalStateWithWindow(). That's probably causing the timers to not fire.

Will look more tomorrow.
Ah. So here's what's _happening_.

When the first prompt is shown, EnterModalState can't figure out the calling window...

6373   JSContext *cx = nsContentUtils::GetCurrentJSContext();
6374 
6375   nsCOMPtr<nsIDOMWindow> callerWin;
6376   nsIScriptContext *scx;
6377   if (cx && (scx = GetScriptContextFromJSContext(cx))) {
6378     scx->EnterModalState();
6379     callerWin = do_QueryInterface(nsJSUtils::GetDynamicScriptGlobal(cx));
6380   }

Specifically, scx is NULL, and so EnterModalState returns a null DOM window.

When the prompt is dismissed, we call...

1492 nsDOMWindowUtils::LeaveModalStateWithWindow(nsIDOMWindow *aWindow)
1493 {
1494   NS_ENSURE_ARG_POINTER(aWindow);
1495   mWindow->LeaveModalState(aWindow);
1496   return NS_OK;
1497 }

aWindow is NULL here, and so we never actually leave the modal state. Thus timeouts are suppressed, and the second prompt is never shown because the timer isn't firing (presumably, I didn't actually verify that).

Relaxing the NS_ENSURE_ARG_POINTER to NS_WARN_IF_FALSE seems to fix things, but II don't think that's actually right. We had to add the "...WithWindow" flavors of these calls to make sure we enter and leave modal state on the right thing, which is slightly tricky because it's called from script (nsPrompter.js). If we're getting a NULL window here, there could be problems with a window improperly receiving events, or with the slow script watchdog getting confused.

Ccing mrbkap for the moment.

Updated

6 years ago
Summary: setTimeout not working after and alert in evalInSandbox → setTimeout doesn't work after calling alert from evalInSandbox

Comment 8

6 years ago
> 6373   JSContext *cx = nsContentUtils::GetCurrentJSContext();

So this is coming back with the sandbox context, right?

The problem is that in this case the script doing the alert is really not running inside a window at all.  It seems to me that in cases like that we should simply use |this| as the window in EnterModalState (and use mContext as scx).
Assignee: nobody → dolske

Comment 9

6 years ago
Do we have a timeline for this being fixed?  Any chance for Firefox 5?
So, I think the fix to this bug is pretty easy, but a little fragile. I agree with bz about using this as the window and mContext as scx. Dolske, we don't actually have to worry about the slow script dialog because, right now, one can't pop up on the sandbox context (there's no context private, meaning that we take the first early return in nsJSContext::DOMOperationCallback).

So, the easiest way to fix this bug would be to use LeaveModalState if EnterModalStateWithWindow returns null.

The only thing I don't really like about that setup is that it hinges on the early return in DOMOperationCallback, but I don't see a really clean way of modeling that relationship.

Comment 11

6 years ago
Add comments in the two places pointing at each other?

Comment 12

6 years ago
Any progress here?  Greasemonkey is still suffering this bug, with an ugly workaround in place.  We've now seen the release of a "major" version (does this make sense any more?) without a fix.

Comment 13

6 years ago
Still wondering when this might be fixed.  Firefox 6?

Comment 14

6 years ago
Anyone fixing this? Firefox 7?

Updated

6 years ago
Duplicate of this bug: 678971

Comment 16

6 years ago
From bug# 678971, this bug also breaks certain websites if a tab modal function is called from scratchpad/webconsole.

Comment 17

6 years ago
I see this on Firefox 8a2 on my page with simple JS. I am not aware of any evalInSandbox, I think I am not using it.
aceman, could you file that a new bug and provide a testcase or URL that demonstrates that behaviour, please? It doesn't necessarily sound like the same bug as this one.

Comment 19

6 years ago
And please mention the new bug's number here?

Comment 20

6 years ago
As described in the original report, FF must get into a state when it stops working. Then after a restart it works again. As always, I am now unable to reproduce it ;) If I come across the state again I will surely try to make a testcase.

Comment 21

6 years ago
Calling the prompt function also does this
Mozilla/5.0 (X11; Linux x86_64; rv:6.0.2) Gecko/20100101 Firefox/6.0.2 ID:20110906071345

Comment 22

6 years ago
(In reply to pqwoerituytrueiwoq from comment #21)
> Calling the prompt function also does this
> Mozilla/5.0 (X11; Linux x86_64; rv:6.0.2) Gecko/20100101 Firefox/6.0.2
> ID:20110906071345

forgot to mention this bug appears to be tab specific once it is triggered it will affect that tab till it is closed or firefox is restarted
Duplicate of this bug: 693197
Bug 693197 is the same issue except with the developer tools instead of evalInSandbox.

Updated

6 years ago
Assignee: dolske → nobody
Blocks: 621764
Component: General → General
Keywords: regression
Product: Firefox → Toolkit
QA Contact: general → general

Comment 25

6 years ago
Any progress on this bug?  Greasemonkey still has an awful alert-only workaround in place.  And users are still hitting and reporting this bug for its other slightly-more-rare variants (prompt):

https://github.com/greasemonkey/greasemonkey/issues/1483

Comment 26

6 years ago
Workaround:
set prompts.tab_modal.enabled to false

Comment 27

6 years ago
Any progress at on this bug?

Comment 28

6 years ago
This bug still seems to be an issue -- just saw someone cussing out Firefox for this in the online forum of a web game whose users frequently employ GM scripts.  :)

Comment 29

5 years ago
We're rapidly coming up on this bug being open for a year.  Will we/when will we see this fixed?

Comment 30

5 years ago
Please fix this! It just affected my script as well. :(

When it gives one alert, my settimeout which is supposed to work every 900 milliseconds in a specific condition randomly fails, silently! :( So bad !! And people curse us script developers. 

I closed and reopened tabs, and the only thing that worked was a browser restart! 

Can someone tell what are the technical difficulties in fixing this?

Updated

5 years ago
Blocks: 59314

Comment 31

5 years ago
I just tried the attached XPI in a trunk nightly, and I see two alerts.  

I also don't see the problem in the Web Console, using the steps from comment 3.

So as far as I can tell, this bug is fixed.  I'll take a quick look into what might have fixed it.

Comment 32

5 years ago
Fix range seems to be http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=5891cc95eac7&tochange=95d1bb200f4e

In that range, bug 754202 seems most likely to have fixed this.  Bobby, does that seem plausible?

Comment 33

5 years ago
This is weird.  We're _still_ pushing the sandbox context (which has a null nsIcriptContext) on the JSContext stack.  So the issue comment 7 describes seems like it should still happen...

Bobby, is something pushing some other JSContext on the stack somewhere here?  Did you change the behavior of nsContentUtils::GetCurrentJSContext?

Comment 34

5 years ago
Ah, looks like the key difference is that LeaveModalStateWithWindow got changed in bug 741587 to not throw on null.

So this is fixed, a far as I can tell.
Depends on: 741587
(In reply to Boris Zbarsky (:bz) from comment #32)
> In that range, bug 754202 seems most likely to have fixed this.  Bobby, does
> that seem plausible?

Those patches were backed out...

Comment 36

5 years ago
> Those patches were backed out...

See comment 34.

Comment 37

5 years ago
Downloaded nightly.  Confirmed that attached test case works as expected.

Installed patched greasemonkey (without our workaround for this bug).  Put in a user script the "not working example" from the original comment.  It works.

Confirmed fixed.

Comment 38

5 years ago
Anthony, thanks for confirming that!
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla16
You need to log in before you can comment on or make changes to this bug.