Do not send extra blur and focus events if popup menu is showing...

Do not send extra blur and focus events if popup menu is showing

Add flag to RenderWidget to remember the popup menu state and
suppress focus / blur events when popup menu is showing. This
fixes the issue that extra focus / blur events are fired after
select control is clicked.

R=darin
BUG=23499
TEST=none



Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=39670

[brettw, 2010-02-18 22:07:36]

Random style drive-by

http://codereview.chromium.org/647047/diff/1/4
File chrome/renderer/render_widget.cc (right):

http://codereview.chromium.org/647047/diff/1/4#newcode341
chrome/renderer/render_widget.cc:341: if (!showing_popup_menu_) {
Don't use {} for single-line conditionals to match the surrounding code.

http://codereview.chromium.org/647047/diff/1/4#newcode879
chrome/renderer/render_widget.cc:879: {
This should go on the previous line.

[Victor Wang, 2010-02-18 22:10:35]

http://codereview.chromium.org/647047/diff/1/4
File chrome/renderer/render_widget.cc (right):

http://codereview.chromium.org/647047/diff/1/4#newcode341
chrome/renderer/render_widget.cc:341: if (!showing_popup_menu_) {
On 2010/02/18 22:07:36, brettw wrote:
> Don't use {} for single-line conditionals to match the surrounding code.

Done.

http://codereview.chromium.org/647047/diff/1/4#newcode879
chrome/renderer/render_widget.cc:879: {
On 2010/02/18 22:07:36, brettw wrote:
> This should go on the previous line.

Done.

[darin (slow to review), 2010-02-19 19:22:46]

LGTM

[darin (slow to review), 2010-02-19 19:23:50]

On 2010/02/19 19:22:46, darin wrote:
> LGTM

Actually, please add some comments to RenderWidget::OnSetFocus to detail why we
sometimes need to suppress focus like this.

[darin (slow to review), 2010-02-19 20:08:56]

Actually, I no longer think this is the correct fix.  More is needed.  If I
give focus to the select, and then click in the location bar, the select
control does not receive a blur event.

-Darin


On Fri, Feb 19, 2010 at 11:23 AM, <darin@chromium.org> wrote:

> On 2010/02/19 19:22:46, darin wrote:
>
>> LGTM
>>
>
> Actually, please add some comments to RenderWidget::OnSetFocus to detail
> why we
> sometimes need to suppress focus like this.
>
>
>
> http://codereview.chromium.org/647047
>

[Victor Wang, 2010-02-20 04:01:58]

looks like the current release version does not fire blur event in this case, I
will take a look to see what can be added to handle this.

On 2010/02/19 20:08:56, darin wrote:
> Actually, I no longer think this is the correct fix.  More is needed.  If I
> give focus to the select, and then click in the location bar, the select
> control does not receive a blur event.
> 
> -Darin
> 
> 
> On Fri, Feb 19, 2010 at 11:23 AM, <mailto:darin@chromium.org> wrote:
> 
> > On 2010/02/19 19:22:46, darin wrote:
> >
> >> LGTM
> >>
> >
> > Actually, please add some comments to RenderWidget::OnSetFocus to detail
> > why we
> > sometimes need to suppress focus like this.
> >
> >
> >
> > http://codereview.chromium.org/647047
> >
>

[darin (slow to review), 2010-02-22 19:52:52]

In that case, feel free to commit this patch as-is, and then work on the
other issue separately.
-Darin

On Fri, Feb 19, 2010 at 8:01 PM, <victorw@chromium.org> wrote:

> looks like the current release version does not fire blur event in this
> case, I
> will take a look to see what can be added to handle this.
>
>
> On 2010/02/19 20:08:56, darin wrote:
>
>> Actually, I no longer think this is the correct fix.  More is needed.  If
>> I
>> give focus to the select, and then click in the location bar, the select
>> control does not receive a blur event.
>>
>
>  -Darin
>>
>
>
>  On Fri, Feb 19, 2010 at 11:23 AM, <mailto:darin@chromium.org> wrote:
>>
>
>  > On 2010/02/19 19:22:46, darin wrote:
>> >
>> >> LGTM
>> >>
>> >
>> > Actually, please add some comments to RenderWidget::OnSetFocus to detail
>> > why we
>> > sometimes need to suppress focus like this.
>> >
>> >
>> >
>> > http://codereview.chromium.org/647047
>> >
>>
>
>
>
>
> http://codereview.chromium.org/647047
>

[Victor Wang, 2010-02-22 22:30:02]

On 2010/02/19 19:23:50, darin wrote:
> On 2010/02/19 19:22:46, darin wrote:
> > LGTM
> 
> Actually, please add some comments to RenderWidget::OnSetFocus to detail why
we
> sometimes need to suppress focus like this.
Done