gadjo Reporter
	Description • 14 years ago

User-Agent:       Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0b8pre) Gecko/20101120 Firefox/4.0b8pre
Build Identifier: 

Invoking alert('test') from firebug console doesn't work. It works fine in firefox console and from url bar

Reproducible: Always

Steps to Reproduce:
1. Install firebug 1.7a7
2. Open firebug console
3. execute alert('test');
Actual Results:  
Nothing happens, only "undefined" is returned as a result

Expected Results:  
Alert box should open

	Nikita Popov
	Comment 1 • 14 years ago

Confirming that alert() does not work. Though prompt() does work. Though confirm() again does not work.

	Justin Dolske [:Dolske] Assignee
	Comment 2 • 14 years ago

All 3 (alert/confirm/prompt) are busted for me. I don't see anything obvious in Firebug's source for these, though they're listed in lib.js's domMemberMap.Window.

	Justin Dolske [:Dolske] Assignee
	Comment 3 • 14 years ago

Hmm. So our code is chugging along fine, and is trying to open a tab-modal prompt. nsPrompter.js's openTabPrompt() runs, but fails in the tabPrompt.appendPrompt() call. Specifically, this code (over in tabbrowser.xml) is throwing a "TypeError: newPrompt.init is not a function" exception. .init() is a function expected to be on the <tabprompt> element, via tabprompts.xml.

I get a couple of warnings in my debug Windows when the code there is calling |stack.appendChild(newPrompt);| -- nsXBLProtoImpl.cpp @ 80, nsXBLService.cpp @ 648... This is the call to newBinding->InstallImplementation(); failing with NS_ERROR_FAILURE. Everything else looks ok to the JS code, except that newPrompt.init == undefined,  so I'm not sure why Firebug is making the binding not apply properly.

	Justin Dolske [:Dolske] Assignee
	Comment 4 • 14 years ago

Calling this a beta8 blocker on the grounds that lots of people probably do debugging-via-alert().

	Justin Dolske [:Dolske] Assignee
	Comment 5 • 14 years ago

Immediate workaround: disable prompts.tab_modal_enabled in about:config, and you'll get working window-modal prompts.

	Justin Dolske [:Dolske] Assignee
	Comment 6 • 14 years ago

(In reply to comment #3)
> "TypeError: newPrompt.init is not a function" exception.

Oh, bah, this was the fault of my debug code -- I made a typo. Ignore most of comment 3. :(

What's _actually_ happening is that we're correctly showing the prompt, but are immediately closing it. My correct-syntax debugging shows that we immediately get triggered by the prompt's "OK" button. Commenting out the addEventListener() for button0 in tabprompts.xml :: init() makes the prompt appear. [Looks like the VK_ENTER / VK_RETURN <handler> is getting called too, ymmv.]

So, I _think_ the problem is that the prompt is getting the "enter" keypress from when you're typing in Firebug's console. Not sure if Firebug is doing something funky here, or we're just managing to create a button fast enough to have it catch that keypress.

A comment in another bug suggested having the prompt ignore keystrokes for a fraction of a second, so that prompts created while typing are a bit harder to dismiss... That's not exactly what's happening here, but might be a (temporary?) hacky workaround.

	gadjo Reporter
	Comment 7 • 14 years ago

If you type javascript:alert(...) in url bar and press enter (and keep the key down) the alert also closes quite fast (and I think it will be much faster when we get rid of the page blurring).

Another thing - try this from firebug: setTimeout("alert('test')", 100)

	Jan Honza Odvarko [:Honza] (always need-info? me)
	Comment 8 • 14 years ago

Just to provide some info how the command line works.

The technique that is used Firebug for executing expression in the page scope is similar to what e10s is planning to do for communication between chrome and content processes (ie using messages).

1) The user types an expression in Firebug's command line and presses enter
2) Firebug is handling keydown
3) Firebug sets the expression string as user data to the current content/tab document (using setUserData)
4) Firebug calls dispatchEvent to send a message to the document
5) The document handles the message (using code injected by Firebug to the page) and eval's the expression.
6) The doc sets the result to the user data and sends message back to Firebug.

Not sure if we can do anything on the Firebug side to fix this (according to comment #6 and comment #7)

Honza

	Boris Zbarsky [:bzbarsky]
	Comment 9 • 14 years ago

> 2) Firebug is handling keydown

Not keypress?  That's most likely the source of the problem on your end, fwiw.  Keydown fires, the alert comes up, then the keypress fires and dismisses the alert.  Probably wasn't an issue with the old alert impl due to some focus shenanigans, I bet.

That said, a simple HTML testcase doesn't seem to show this problem (but it's using space, since enter doesn't work on Mac to close alerts at the moment):

  <body>
    Click in the input, then place space.  You should see an alert.
    <input onkeydown="alert('xxx')">
  </body>

	gadjo Reporter
	Comment 10 • 14 years ago

(In reply to comment #9)
>   <body>
>     Click in the input, then place space.  You should see an alert.
>     <input onkeydown="alert('xxx')">
>   </body>

Win7 - I can see alert after pressing space, but not after enter or esc

I think that in the code below onKeyAction should do something only when keydown event has been fired after initializing the prompt:

+        <handler event="keypress" keycode="VK_ENTER"
+                 group="system" action="this.onKeyAction('default', event);"/>
+        <handler event="keypress" keycode="VK_RETURN"
+                 group="system" action="this.onKeyAction('default', event);"/>
+        <handler event="keypress" keycode="VK_ESCAPE"
+                 group="system" action="this.onKeyAction('cancel', event);"/>

Explanation is quite simple - let's assume that alert is run from firebug by pressing enter, alert shows, user releases the key, keypress event fires, alert doesn't close because keydown event wasn't fired yet (it was, but before opening the alert); then user presses enter again => keydown (ok, now we accept key events) => keypress => close

	Nikita Popov
	Comment 11 • 14 years ago

Confirming bz's guess:

data:text/html,<input onkeydown="alert('Test')" value="onkeydown"><input onkeypress="alert('Test')" value="onkeypress">

When pressing <Enter> in the first (onkeydown) nothing happens,
when pressing <Enter> in the second (onkeypress) everything works fine.

(Tested on Windows)

	Jan Honza Odvarko [:Honza] (always need-info? me)
	Comment 12 • 14 years ago

Thanks for the tip! The keydown vs. keypress events look like the problem.

Anyaway, just to note that when testing Firebug with Firefox 3.6 the alert() works well. Is that expected behavior in Fx 40?

Honza

	Justin Dolske [:Dolske] Assignee
	Comment 13 • 14 years ago

(In reply to comment #12)
> Thanks for the tip! The keydown vs. keypress events look like the problem.
> 
> Anyaway, just to note that when testing Firebug with Firefox 3.6 the alert()
> works well. Is that expected behavior in Fx 40?

I think bz is saying in comment 9 that this just happens to work accidentally. Lucky for you. :)

Is Firebug going to change to use keypress? That seems like the right thing to do anyway.

Though I suppose we should still ship B8 with the workaround I mention in comment 6, patch in a moment.

	Justin Dolske [:Dolske] Assignee
	Comment 14 • 14 years ago

This makes the Firebug testcase WFM. The 100ms delay is small enough that the prompt is still easy to quickly dismiss, but large enough to avoid the keydown/keypress problem.

I'm not particularly thrilled with having random tests needing to unset .ignoreCommands (which is needed to make tests pass reliably, otherwise they don't have a good way to know when the prompt is able to take commands). But I guess it's not _too_ bad. The only somewhat better idea is to have CommonDialog.jsm pass an onPromptReady callback [which would fire the tabmodal/common-dialog-loaded notification] to the prompt's init(), which it could then defer calling until the setTimeout fires.

	Justin Dolske [:Dolske] Assignee
	Comment 15 • 14 years ago

Ahem.

	Jan Honza Odvarko [:Honza] (always need-info? me)
	Comment 16 • 14 years ago

Patch for Firebug committed into 1.7 branch.
http://code.google.com/p/fbug/source/detail?r=8493

Honza

	Dave Webber
	Comment 17 • 14 years ago

I just noticed that app-modal dialogues opened from the address bar in Firefox 3.6 or 4 (at least on Mac OS X) close if you hesitate release the return/enter key. I think the only reason I've never noticed this before has been because the responsiveness of the alert is delayed by the animation of its appearance, and I type faster than that. 

Come to think of it, that could be an accessibility issue for users who are slow to release keys (as due to a mobility impairment). Given that the new tab-modal dialogues appear with no more delay than that provided by rendering the blurring effect, this may become more of an accessibility issue for users who don't release each key they press without hesitation. Firebug issues aside, try just typing javascript:alert('test') into the address bar, and keeping the return/enter key pressed for a moment.

Perhaps it would be good to ignore key-action events which started before the dialogue opened, as mentioned in comment 10, or (better?) to ignore such events if they occur before a delay equal to the user's key initial key repeat delay [which is stored as a number of 15ms(?) intervals in the global preference "InitialKeyRepeat" on Mac OS X, and as a number ms in the "AutoRepeatDelay" key in HKCU\Control Panel\Accessibility\Keyboard Response on Windows; and which IIUC is backfilled as a number of ms into the third argument of XkbGetAutoRepeatRate on Linux (when using X11?)].

	Dave Webber
	Comment 18 • 14 years ago

Sorry, I missed that much of my previous comment had already been covered in comment 7, and another part [less specifically] in comment 6. At least there's still the potential accessibility concern and a non-arbitrary source from which to determine the delay (while presumably also implicitly resolving the potential accessibility issue) that's not quite redundant.

	gadjo Reporter
	Comment 19 • 14 years ago

(In reply to comment #17)
> or (better?) to ignore such events
> if they occur before a delay equal to the user's key initial key repeat delay
> [which is stored as a number of 15ms(?) intervals in the global preference
> "InitialKeyRepeat" on Mac OS X, and as a number ms in the "AutoRepeatDelay" key
> in HKCU\Control Panel\Accessibility\Keyboard Response on Windows; and which
> IIUC is backfilled as a number of ms into the third argument of
> XkbGetAutoRepeatRate on Linux (when using X11?)].

This is equal to saying that if the key event started before opening the dialog we should accept the next keypress event as it will be fired after the key repeat delay and there is no need to retrieve this key repeat delay value from the OS API.
If the key event started after opening the dialog there is no problem and we accept the first key press event.
And IMHO it's the best solution.

	Justin Dolske [:Dolske] Assignee
	Comment 20 • 14 years ago

Downgrading this from a Beta 8 blocker; my initial fear ion comment 4 was that Firebug was breaking all prompts and/or doing something deeply scary. But we've since found it's only an issue for prompts manually invoked by entering code in the FB console, and that it's just a timing issue with key events.

This bug will probably pick up some dupes if Firebug doesn't ship it's fix before beta 8, but neither Firefox nor Firebug will be significantly broken.

	Jan Honza Odvarko [:Honza] (always need-info? me)
	Comment 21 • 14 years ago

Firebug is going to ship the patch (mentioned in comment 16) in version 1.7a8 which should be released yet before beta 8.
Honza

	Justin Dolske [:Dolske] Assignee
	Comment 22 • 14 years ago

Let's just call this WONTFIX... It's a fairly minor issue that's going to be fixed from Firebug's side.