Prevent overlay enter events from executing two buttons.

Prevent overlay enter events from executing two buttons.

If the focus before opening an overlay was on a button, the overlay
will restore focus to that button when the overlay closes. If the
cause of the overlay closing was executing a default button via the
document handler, the newly focused button would then execute
immediately via the same event. This CL prevents that from
happening by calling preventDefualt() after executing the defualt
button.

BUG=268336
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=215826

[Dan Beam, 2013-08-05 23:34:24]

lgtm, but can haz test?

it's pretty hard to keep track of all the manual focus management magic going on
these days...

[Rune Fevang, 2013-08-05 23:56:12]

On 2013/08/05 23:34:24, Dan Beam wrote:
> lgtm, but can haz test?
> 
> it's pretty hard to keep track of all the manual focus management magic going
on
> these days...

I'm not quite sure how to write one. The event that happens on the second button
is an onclick, and not the same as the onkeydown event. The issue doesn't even
reproduce if you put a breakpoint in the handler... In the other CLs I
dispatched keydown events on the elements in question, but doing that never
triggers the default action in the first place, so it won't work in this case.

[Dan Beam, 2013-08-06 00:06:31]

On 2013/08/05 23:56:12, Rune Fevang wrote:
> On 2013/08/05 23:34:24, Dan Beam wrote:
> > lgtm, but can haz test?
> > 
> > it's pretty hard to keep track of all the manual focus management magic
going
> on
> > these days...
> 
> I'm not quite sure how to write one. The event that happens on the second
button
> is an onclick, and not the same as the onkeydown event. The issue doesn't even
> reproduce if you put a breakpoint in the handler... In the other CLs I
> dispatched keydown events on the elements in question, but doing that never
> triggers the default action in the first place, so it won't work in this case.

maybe just check the default is prevented?

[Rune Fevang, 2013-08-06 00:47:41]

On 2013/08/06 00:06:31, Dan Beam wrote:
> On 2013/08/05 23:56:12, Rune Fevang wrote:
> > On 2013/08/05 23:34:24, Dan Beam wrote:
> > > lgtm, but can haz test?
> > > 
> > > it's pretty hard to keep track of all the manual focus management magic
> going
> > on
> > > these days...
> > 
> > I'm not quite sure how to write one. The event that happens on the second
> button
> > is an onclick, and not the same as the onkeydown event. The issue doesn't
even
> > reproduce if you put a breakpoint in the handler... In the other CLs I
> > dispatched keydown events on the elements in question, but doing that never
> > triggers the default action in the first place, so it won't work in this
case.
> 
> maybe just check the default is prevented?

Done.

[Dan Beam, 2013-08-06 00:53:31]

slgtm

https://codereview.chromium.org/22268005/diff/7001/chrome/browser/ui/webui/op...
File chrome/browser/ui/webui/options/options_browsertest.js (right):

https://codereview.chromium.org/22268005/diff/7001/chrome/browser/ui/webui/op...
chrome/browser/ui/webui/options/options_browsertest.js:236: // the default
button.
nit: explain why you want to ensure preventDefault() is called (which bug this
fixes or behavior it introduces)

[Rune Fevang, 2013-08-06 00:58:02]

https://codereview.chromium.org/22268005/diff/7001/chrome/browser/ui/webui/op...
File chrome/browser/ui/webui/options/options_browsertest.js (right):

https://codereview.chromium.org/22268005/diff/7001/chrome/browser/ui/webui/op...
chrome/browser/ui/webui/options/options_browsertest.js:236: // the default
button.
On 2013/08/06 00:53:32, Dan Beam wrote:
> nit: explain why you want to ensure preventDefault() is called (which bug this
> fixes or behavior it introduces)

Done.

[commit-bot: I haz the power, 2013-08-06 01:01:52]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/rfevang@chromium.org/22268005/13001

[commit-bot: I haz the power, 2013-08-06 06:31:12]

Change committed as 215826