294950 - Page receives return key events intended for location bar

Status: RESOLVED WORKSFORME

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

Description

[Daniel Veditz [:dveditz]]

In Firefox 1.0.4 (but not earlier versions) web content can capture the return
key intended for the location bar, making it difficult to go to another site.
Other key events don't leak into the page (which is good since that'd be a
serious privacy worry). As a DOS it's not all that effective (open new tab or
window, abandon and close stuck one) and even if fixed the same kind of thing
could be accomplished with onunload.

Since so little went into 1.0.4 it might be worth figuring out what caused this
change. Darin did make the following change to fix a crash in the event state
manager:
http://tinderbox.mozilla.org/bonsai/cvsview2.cgi?diff_mode=context&whitespace_mode=show&subdir=mozilla/content/events/src&command=DIFF_FRAMESET&file=nsEventStateManager.cpp&rev1=1.487.2.1.2.12&rev2=1.487.2.1.2.12.2.1&root=/cvsroot

The odd thing is that the suite appears to have already behaved this way for a
while, with identical(?) back-end code.

Works OK:   Firefox 1.0, 1.0.1, 1.0.2, 1.0.3
Sees event: Firefox 1.0.4, Suite 1.7.5, Suite 1.7.7, trunk Suite

Comment 1

[Daniel Veditz [:dveditz]]

Attachment: testcase

Comment 2

[Daniel Veditz [:dveditz]]

This regression was noted by Renee Trisberg of Hansabank, who provided this
testcase in mail to [e-mail]

Comment 3

[Boris Zbarsky [:bzbarsky]]

So... if this is not a problem on the 1.8 branch, I think we should just WONTFIX this.

Comment 4

[Pavel Penaz]

With Firefox 2.0 I can still reproduce this bug using the attached testcase.

Mozilla/5.0 (Windows; U; Windows NT 5.1; cs; rv:1.8.1) Gecko/20061010 Firefox/2.0

Comment 5

[Boris Zbarsky [:bzbarsky]]

OK.  So I have managed to reproduce this on Linux.  What happens there is that the keypress handler calls BrowserLoadURL, which calls focus() on the content window after starting the URL load.

Then the native keyup code calls key_release_event_cb which does:

  nsWindow *focusWindow = gFocusWindow ? gFocusWindow : window;
  return focusWindow->OnKeyReleaseEvent(widget, event);

and since gFocusWindow is now the content window, that's where the keyup event goes.

I wouldn't be surprised if something similar happens on Windows, either in the Windows widget code or in the OS widgetry itself.

I wonder whether we could avoid doing the sync focus() call here...

And even more interestingly, what changed on branch to cause this.  Certainly the focus() call has been there all along....

Do we have dates for 1.0.3 and 1.0.4 somewhere?

Comment 6

[:Gavin Sharp [email: [e-mail]]]

(In reply to comment #5)
> I wonder whether we could avoid doing the sync focus() call here...

We probably could, but it seems somewhat fragile to rely on no one playing with focus to ensure that events are dispatched properly. I don't really know whether that's feasible or not.

Comment 7

[Boris Zbarsky [:bzbarsky]]

It's not clear how else we can dispatch them.  By the time the keyup happens, the focused thing is the content area, so the keyup should be dispatched to the content area, no?

Comment 8

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

Could we delay the focus() call until the keyup is received?

Comment 9

[:Gavin Sharp [email: [e-mail]]]

(In reply to comment #7)
> It's not clear how else we can dispatch them.  By the time the keyup happens,
> the focused thing is the content area, so the keyup should be dispatched to the
> content area, no?

Oh, I see. I'm not sure how to best fix this, then. We start the new load when we get a keypress event for DOM_VK_RETURN, and we want the content area focused "after that". We could probably move the focus call to a keyup handler for the location bar, I guess, and be sure not to change focus in any way before then.

For what it's worth, I don't think the regression range in this bug is correct. I can reproduce the issue in 1.0.3 and 1.0.4, and even as far back as a trunk build from 2004-02-10.

Comment 10

[:Gavin Sharp [email: [e-mail]]]

Attachment: simpler testcase

Comment 11

[Justin [:JW_SoftvisionQA]]

I can not reproduce this issue on:

 	Mozilla/5.0 (Windows NT 6.3; WOW64; rv:45.0) Gecko/20100101 Firefox/45.0
 	Mozilla/5.0 (Windows NT 6.3; WOW64; rv:48.0) Gecko/20100101 Firefox/48.0

I will close this issue as works for me. If anyone can reproduce, please feel free to reopen and post your str.