239078 - Hit Enter/Return focus a button doesn't respond when <base target ="_blank">

Status: RESOLVED FIXED

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

Description

[Yonghao Wang]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.7b) Gecko/20040316
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.7b) Gecko/20040316

if you have a webpage contains <base target ="_blank"> and then when you focus
on a button which suppose to open a page (new page should) then you can click
this button and open new page in new window, however you cannot hit key
"return/enter" to make it happen.

Reproducible: Always
Steps to Reproduce:
1.
2.
3.

Comment 1

[Boris Zbarsky [:bzbarsky]]

danm, this is a regression from your event filtering landing for the popup
blocker.  The problem is that the code in nsDocShell::FindTarget calls Open() on
the window to create the new window.

This situation was discussed in bug 197919, but dropped because it doesn't
happen for <a href> links. Unfortunately, it _does_ happen for buttons.

We can't just bypass the popup blocker at that callsite because that would open
up the "call form.submit() to create popups" holes.

So I think we do in fact want to either special-case some key events (enter,
space, etc) or to change the form code to synthesize click events like links do
or something...

Comment 2

[Dan M]

Crikey! Event handling is utterly, completely different for links and buttons! I
didn't even think to check that. Well in the long run a consistent event story
and even better, a way to tell whether an event was initiated by a user action,
would be helpful for problems like this and like bug 101190. In the less-long
run I suppose we could special-case the keypress event.

Comment 3

[Dan M]

Attachment: special-case space and return keys in popup suppressor

Color me unhappy. Return isn't so bad, but space...

Comment 4

[Dan M]

Attachment: completely unfetter windows from return and space key handlers

Oh ugh. Actually these events should create non-popup windows, just like
mouseclick.

Comment 5

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 145106 [details] [diff] [review]
completely unfetter windows from return and space key handlers

So we fire buttons on space keyup and enter keypress?  Ugh.  What about <button
type="submit"> vs <input type="submit">?  Is there a difference in how
space/enter are handled?

Also, what about <input type="image">?	And <area>?

Finally, for ultimate confusion, what about

<input type="submit" accesskey="x">

when the user hits "alt-x"?

I guess this last case is rare enough that we can do it as part of a general
"the event was triggered by the user" thing, eh?

If the the various things space/enter can trigger (listed above) are all fine
with this patch, r=bzbarsky.

Comment 6

[Dan M]

Isn't that nice? Even space and return are treated differently.

I've tried <a> elements, <input type=button,checkbox,submit,img>, <map> and
simple XUL <button>s. Everything seems to fall into two classes: anchors and
maps are asynchronous with no DOM event in evidence at time of processing. Every
other element presents an event. I think we're covered with this patch except
for accesskeys, as you pointed out. For those, I suppose it's not much worse to
make an exception of the Alt key as well. This works:

PRUint32 key = NS_STATIC_CAST(nsKeyEvent *, currentEvent)->keyCode;
PRBool isAlt = NS_STATIC_CAST(nsKeyEvent *, currentEvent)->isAlt;
...
  case NS_KEY_PRESS :
    // return key on focused button. see notes at NS_MOUSE_LEFT_CLICK.
    if (key == nsIDOMKeyEvent::DOM_VK_RETURN || isAlt)
      abuse = openAllowed;

Comment 7

[Boris Zbarsky [:bzbarsky]]

Note that we could conceivably make form submits asynchronous like link
clicks... perhaps only in cases when they are not triggered by form.submit()?

Comment 8

[Dan M]

But that wouldn't help the non-submit button
<input type=button value="click me" accesskey="c" onclick="openWindow()">
and it would hurt the ability to explicitly control submit handlers. (Though
that doesn't seem to be working at all right now! I no longer see form submit
events in yesterday's build. &*$#&#%#@!)

Sigh. Ignoring that for now. OK, as the patch stands, all form elements will be
able to open a new window when focused and triggered from the keyboard using the
default action key, but they can't open a new window when triggered using the
accesskey. Going for sr=.

Comment 9

[Johnny Stenback (:jst)]

Comment on attachment 145106 [details] [diff] [review]
completely unfetter windows from return and space key handlers

sr=jst

Comment 10

[chris hofmann]

Comment on attachment 145106 [details] [diff] [review]
completely unfetter windows from return and space key handlers

a=chofmann for 1.7

Comment 11

[Dan M]

checked in to trunk for 1.7