230346 - Using the spacebar to dismiss JavaScript sheets sends key events to the content

Status: NEW

(Core :: DOM: UI Events & Focus Handling, defect)

Description

[Nathaniel Madura]

User-Agent:       Mozilla/5.0 (Macintosh; U; PPC Mac OS X; en-US; rv:1.0.1) Gecko/20021104 Chimera/0.6
Build Identifier: Mozilla/5.0 (Macintosh; U; PPC Mac OS X; en-US; rv:1.0.1) Gecko/20021104 Chimera/0.6

full keyboard access (which can be enabled in the keyboard pref pane) allows you
to select (dialog) buttons using the space key.
However, with java Dialogs (they look roughly like this...) 

[Javascript Application]
then some message here?
   cancel   ok

hitting the space bar makes then dialog go away, and then it immediately comes
back, infinitely (?). return does produce the correct response. 

Reproducible: Always

Steps to Reproduce:




I think this happened in Jaguar too, though I am not using jaguar anymore on any
of my computers so I can't verify it.

Comment 1

[Ludovic Hirlimann [:Usul]]

Is this a duplicate of 163468 ?

Comment 2

[Nathaniel Madura]

Attachment: Screenshot of the problem

With full keyboard access enabled there would normally be a light blue line
around the OK button, to simulate a click you "should" be able to hit the space
bar. When this is done the dialog in the screenshot goes away, but then comes
back.. until you use the enter key or you click on ok.

Comment 3

[Nathaniel Madura]

I belive this is different from 163468, 163468 seems to be not being able to
type into java applications within the browser. This is more Dialog control,
however this problem only seems to happen with Java Dialogs. I am fairly certain
that all the "regular" camino dialogs behave correctly.

Comment 4

[Greg K.]

Nathaniel, please provide a testcase URL.

Comment 5

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

I can replicate this on 20041012/0.8+ nightly, OS X 10.3.5.  This bug is
confusing to wrap one's mind around, so I will try to lay out a better series of
steps.  And this bug is discussing *JavaScript*-generated dialogues, not Java.

Steps to reproduce:

1.  Enable "Full Keyboard Access" in the Keyboard Shortcuts section of the
Keyboard & Mouse system prefpane.

2.  Read the article "Full Keyboard Access shortcuts for selecting an action" in
Help Viewer to familiarize oneself with what the spacebar is supposed to do.

3.  Find a page that has a JavaScript-generated dialogue box (the only one I'm
familiar with is the "Confirm empty trash" JS dialogue on webmail.earthlink.net;
if I come across a "public" JS dialogue in my surfing I'll post the url, or
probably anyone who knows JS--not me :(--can write a simple test script) and
perform the action that invokes the dialogue/sheet.

4.  Initial state of dialogue: "OK" is both colored (default) and has "focus
ring" from full keyboard access.  Hit tab to move focus ring to "Cancel" button
(see following attachment for visual of dialogue in new state).

5.  Hit the space bar.

6.  The dialogue goes away and immediately retuns.

Expected result: 

In step 5, according to the Help document cited above, the space bar is supposed
to select the highlighted item (as opposed to return, which selects the default
item...), which is this case would cancel the dialogue/sheet for good.  It's not
clear to me what Camino is actually doing (not selecting cancel, because
selecting cancel via mouse or Esc does actually make the dialogue/sheet go away
for good).

In this particular JS dialogue, there's an easy work-around (Esc if one wants to
cancel or Return if one wants to proceed) but one imagines there might be a JS
dialogue somewhere that has three buttons or where the button that one tabs to
in order to select is not cancel...

Hope this helps the triage.

Comment 6

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

Attachment: New state of JS dialogue (rf Comment 5, step 4)

The dialogue mentioned in comment 5, showing the state of the dialogue at the
end of step 4--after one tabs to focus/highlight the cancel button.

Comment 7

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

Someone with the power to do so should change the summary (Java => JavaScript);
 adding myself to cc:

Comment 8

[Jasper]

Changed the summarary, I now just need a testcase. Could somebody please provide
that.

Comment 9

[Jasper]

Attachment: Testcase

This basic tescase shows that Camino does the correct thing.

- open the test case.
- hitting spacebar "Select the highlighted item" which is exactly the same as
submitting or hitting the selected button. using the tab key you can highlight
the other buttons.
- the alert sheet diasppears as it should when hitting the spacebar.

I checked with other applications and Camino has the exact same behaviour as
any other application I tested on.

Comment 10

[Jasper]

Marking WONTFIX as we do the right thing.

Comment 11

[Nathaniel Madura]

This problem is not resolved... to reproduce.. 
1. sign up for a yahoo email account
2. get several emails... maybe wait for some spam
3. in your inbox or bulk mail folder, select multiple items for deletion and
click delete
4. Javascript pop-up window opens
5. hit space bar
6. hit space bar

some times it goes away after the second space bar push, sometimes it continues
much longer!

Comment 12

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

There must be something more to the problem that makes it manifest sometimes and
not others.  

I can no longer reproduce it at my webmail (where earlier I was able to do so)
or at my Yahoo mail--not with 20041019 0.8+ nor with the released 0.8.1!

[e-mail], are you using a current build?

Comment 13

[Nathaniel Madura]

I have reproduced the problem on 3 computers in the last 12 hours, on an iBook
running Camino 0.8.1, on a powerbook running Camino 2004100908 and most recently
on a powermac G5 2004102108.
On the G5 I captured a video of the problem, see
http://blatantmisuse.com/JavaScriptFullKeyboardAccessBug.mov
the file is about 1.8 MB 

Comment 14

[Jasper]

The movie is very nice but not helpful at all as the textcase I created works
fine for me. 

Does the attached test case work for you?
Could you please create a page that contains the javascript on that page. Or
perhaps save that page and attach it to this bug. It could just aswell be the
javascript only licstening for a mouse click and not for other key events. More
info...

Comment 15

[Nathaniel Madura]

Attachment: TestCase 2

Attached is a test case that causes the repeat problem on all the computers I
have  Camino installed on.

Comment 16

[Jasper]

Thanks Nathaniel, that is a very good test case. Confirming using 2004102108
(v0.8+) NB.

I'd like to know if this is something caused by the script or us now :)

Comment 17

[L. H.]

The problem seem to be the exact key events. For space the sheet button takes
keydown but the button takes keyup, so the sheet is closed by the first half and
the button is pressed by the second half. For return both seem to take key down.

space:
open test case
tab to focus button
hold down space
release space - button is pressed
tab to OK
hold down space bar - sheet goes away
release space bar - sheet reappears because button is pressed

return:
open test case
tab to focus button
hold down return - button and sheet cycle quickly

Comment 18

[Mike Pinkerton (not reading bugmail)]

what's the thought here? change how gecko handles a11y key presses on buttons?

Comment 19

[Simon Fraser [no longer active]]

Related to bug 179307?

Comment 20

[Simon Fraser [no longer active]]

I think we should somehow eat the keyUp event after the keyDown that dimisses
the sheet, or remove it from the event queue.

Comment 21

[Samuel Sidler (old account; do not CC)]

This seems like an accessiblity thing as well if a user can't dismiss a dialog from the keyboard.

I'd, personally, like this for 1.0.

Comment 22

[Samuel Sidler (old account; do not CC)]

-> 1.1

Comment 23

[Chris Lawson (gone)]

(In reply to comment #21)
> This seems like an accessiblity thing as well if a user can't dismiss a dialog
> from the keyboard.

Is there a reason the Escape key *doesn't* work in these situations? I understand the bug is annoying to those who are used to the space bar selecting the highlighted option, but don't Esc and Return work properly as keyboard shortcuts for Cancel and OK?

cl

Comment 24

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

Mass un-setting milestone per 1.6 roadmap.

Filter on RemoveRedonkulousBuglist to remove bugspam.

Developers: if you have a patch in hand for one of these bugs, you may pull the bug back to 1.6 *at that point*.

Comment 25

[Mark Mentovai]

Attachment: Testcase 3 (like 2 but with key event logging)

This is similar to testcase 2 (attachment 163168 [details]), but I've added the logging from bug 26269 attachment 230983 [details].

Comment 26

[Mark Mentovai]

Gecko triggers buttons on space intentionally.

http://bonsai.mozilla.org/cvsblame.cgi?file=/mozilla/content/html/content/src/nsHTMLButtonElement.cpp&rev=1.166&mark=386,388-389,393-394#383
http://bonsai.mozilla.org/cvsblame.cgi?file=/mozilla/content/html/content/src/nsHTMLInputElement.cpp&rev=1.470&mark=1707,1709-1710,1715-1716#1706

<button> and <input> were made to trigger on space-key-up in bug 68850.  It was intentional: Gecko buttons are apparently supposed to trigger on space-up.  Unfortunately, Mac buttons trigger on space-down.  In this bug, the dialog eats the space-down, and then Gecko gets the space-up, so what's a single keypress and release to the user winds up triggering two distinct actions.

Someone asked in bug 68850 comment 2 if the behavior was right for all platforms.  There was no response.  It's not right on the Mac.

Comment 27

[Mark Mentovai]

Attachment: Make Gecko buttons trigger on space-down instead of space-up with Cocoa

Comment 28

[Mark Mentovai]

Moving the bug to the proper component, although the specific manifestation in this bug is Camino-only.

Comment 29

[Mark Mentovai]

Attachment: Make buttons trigger on space-down instead of space-up with Cocoa, v2 handles XUL buttons too

Comment 30

[Josh Aas]

Comment on attachment 293567 [details] [diff] [review]
Make buttons trigger on space-down instead of space-up with Cocoa, v2 handles XUL buttons too

This behavioral change is specific to Mac OS X, not the Mac OS X API we happen to be using. Please change

+#ifndef MOZ_WIDGET_COCOA

to use XP_MACOSX.

Comment 31

[Mark Mentovai]

Comment on attachment 293567 [details] [diff] [review]
Make buttons trigger on space-down instead of space-up with Cocoa, v2 handles XUL buttons too

You're right, for the trunk.  But I think I want to handle this a little bit differently: set :hover:active on keydown and trigger the action on keyup if :hover:active is set.  This is what we currently do for XUL buttons, and it's a lot nicer - we should do the same for HTML buttons.  That would be a cross-platform change.  On the branch, I'd still want to take that change as a MOZ_WIDGET_COCOA-only change for Camino only, to avoid changing the behavior for other products.

I'm canceling the review request until I can investigate the :hover:active thing more closely.

Comment 32

[Josh Aas]

Right, I assumed your patch was a trunk patch. Am I wrong?

Comment 33

[Mark Mentovai]

The patch itself was supposed be both trunk and branch, but I used MOZ_WIDGET_COCOA because I couldn't use XP_MACOSX on the branch.

I have a new patch (to be attached shortly) that actually fixes this up for all platforms, and is intended for the trunk without any ifdefs.  I'd still like it on the branch as a Camino-only (#ifdef MOZ_WIDGET_COCOA) patch.

Comment 34

[Mark Mentovai]

Attachment: Set :active:hover on space bar down and trigger on space bar up, all platforms

This keeps HTML <button> and <input> elements triggering on space-up as they currently do on all platforms, but only if they saw a space-down.  The :active:hover state is set on space-down and checked (and cleared) on space-up.  This is exactly how XUL buttons work:

http://bonsai.mozilla.org/cvsblame.cgi?file=/mozilla/layout/xul/base/src/nsButtonBoxFrame.cpp&rev=1.45&mark=87-90,115-123#77

roc, is this something we can/should do in content too?  It doesn't seem to have any content-specific problems, and it gives closer parity between XUL UI elements and HTML content elements.

This is targeted at the trunk (1.9 branch) but in an #ifdef MOZ_WIDGET_COCOA form to make it Camino-only would fix the Camino bug as described in comment 0 on the 1.8.1 branch as well.

Comment 35

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Setting HOVER manually like this sounds dodgy to me. For one thing, if it's already hovered, aren't you toggle hover *off*? Likewise if someone insane is clicking on the button to make it ACTIVE and they press space as well.

I suggest you ditch setting HOVER altogether. You probably should refactor the code that sets ACTIVE for both key presses and mouse clicks so that, at least, doing a keydown or a mousedown while the button is already active doesn't make it inactive.

smaug should really look at this too.

Comment 36

[Mark Mentovai]

Comment on attachment 297066 [details] [diff] [review]
Set :active:hover on space bar down and trigger on space bar up, all platforms

I used :active:hover because that's what XUL buttons do - just like what's done in this patch.  I don't think EventStateManager::SetContentState toggles :hover off if it's already on, I think it ensures :hover is on.  In any case, if you insist, I can remove :hover.

smaug, what do you think?

Comment 37

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

I think only :active state should be set, not :hover. If mouse is over
something else which has its own :hover style, that shouldn't be cleared.
Bug in XUL implementation, IMO, but not sure if we want to change it.

How does the :active state get cleared if .preventDefault() is called during keyup event dispatch? Or do you think deactivating ::active is the 
default action in this case? I doubt other browsers handle it that way.
Do you really want to activate :active also if content is dispatching (untrusted) 
keydown events. Note XUL key event handling happens in frame code, so events should be trusted (in this key event case).

Comment 38

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

The bug assignee is inactive on Bugzilla, so the assignee is being reset.