Hide overflow menu/closed caption list when clicking outside the list

Hide overflow menu/closed caption list when clicking outside the list

BUG=651431
Committed: https://crrev.com/2569ec9714c71a31f0135f55913a5e40c493e161
Cr-Commit-Position: refs/heads/master@{#422752}

[Zhiqiang Zhang (Slow), 2016-09-30 14:54:25]

zqzhang@chromium.org changed reviewers:
+ mlamouri@chromium.org

[mlamouri (slow - plz ping), 2016-10-01 16:02:49]

Excellent! :) I'm wondering if instead of two callbacks we could have one and we
would basically try to close all the overflow menus if a click happens. It would
simplify the logic (no need to keep track of the state). It means that we are
listening for an event when we don't need it but it might be an insignificant
overhead. WDYT?

[Zhiqiang Zhang (Slow), 2016-10-03 10:05:54]

The CQ bit was checked by zqzhang@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-10-03 10:06:15]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Zhiqiang Zhang (Slow), 2016-10-03 10:06:36]

PTAL

On 2016/10/01 16:02:49, mlamouri wrote:
> Excellent! :) I'm wondering if instead of two callbacks we could have one and
we
> would basically try to close all the overflow menus if a click happens. It
would
> simplify the logic (no need to keep track of the state). It means that we are
> listening for an event when we don't need it but it might be an insignificant
> overhead. WDYT?

Done.

[commit-bot: I haz the power, 2016-10-03 11:25:19]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-10-03 11:25:20]

Dry run: This issue passed the CQ dry run.

[Zhiqiang Zhang (Slow), 2016-10-03 13:04:55]

Description was changed from

==========
Hide overflow button/closed caption list when clicking outside the list

BUG=651431
==========

to

==========
Hide overflow menu/closed caption list when clicking outside the list

BUG=651431
==========

[Zhiqiang Zhang (Slow), 2016-10-03 18:13:57]

PTAL

[mlamouri (slow - plz ping), 2016-10-03 18:50:20]

lgtm with comments applied :)

https://codereview.chromium.org/2382233002/diff/40001/third_party/WebKit/Sour...
File
third_party/WebKit/Source/core/html/shadow/MediaControlsWindowEventListener.cpp
(right):

https://codereview.chromium.org/2382233002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/html/shadow/MediaControlsWindowEventListener.cpp:52:
}
style: remove {}

https://codereview.chromium.org/2382233002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/html/shadow/MediaControlsWindowEventListener.cpp:59:
if (event->type() == EventTypeNames::click)
DCHECK_EQ(event->type, EventTypeNames::click);
handleClickEvent();

https://codereview.chromium.org/2382233002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/html/shadow/MediaControlsWindowEventListener.cpp:69:
m_isActive = false;
Should this be done in `stop()` instead of here?

[Zhiqiang Zhang (Slow), 2016-10-04 09:25:18]

The CQ bit was checked by zqzhang@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-10-04 09:25:40]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Zhiqiang Zhang (Slow), 2016-10-04 09:27:08]

zqzhang@chromium.org changed reviewers:
+ foolip@chromium.org

[Zhiqiang Zhang (Slow), 2016-10-04 09:31:27]

https://codereview.chromium.org/2382233002/diff/40001/third_party/WebKit/Sour...
File
third_party/WebKit/Source/core/html/shadow/MediaControlsWindowEventListener.cpp
(right):

https://codereview.chromium.org/2382233002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/html/shadow/MediaControlsWindowEventListener.cpp:52:
}
On 2016/10/03 18:50:20, mlamouri wrote:
> style: remove {}

Done.

https://codereview.chromium.org/2382233002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/html/shadow/MediaControlsWindowEventListener.cpp:59:
if (event->type() == EventTypeNames::click)
On 2016/10/03 18:50:19, mlamouri wrote:
> DCHECK_EQ(event->type, EventTypeNames::click);
> handleClickEvent();

Done.

https://codereview.chromium.org/2382233002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/html/shadow/MediaControlsWindowEventListener.cpp:69:
m_isActive = false;
On 2016/10/03 18:50:19, mlamouri wrote:
> Should this be done in `stop()` instead of here?

Done.

[commit-bot: I haz the power, 2016-10-04 11:00:57]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-10-04 11:00:58]

Dry run: This issue passed the CQ dry run.

[foolip, 2016-10-04 11:03:25]

rs lgtm, but if you haven't already, please investigate if you can use loss of
focus as the trigger, i.e. something involving the blur event.

[Zhiqiang Zhang (Slow), 2016-10-04 11:14:51]

On 2016/10/04 11:03:25, foolip wrote:
> rs lgtm, but if you haven't already, please investigate if you can use loss of
> focus as the trigger, i.e. something involving the blur event.

We are going to look into it. Thanks!

[Zhiqiang Zhang (Slow), 2016-10-04 11:15:05]

The CQ bit was checked by zqzhang@chromium.org

[Zhiqiang Zhang (Slow), 2016-10-04 11:15:06]

The patchset sent to the CQ was uploaded after l-g-t-m from
mlamouri@chromium.org
Link to the patchset: https://codereview.chromium.org/2382233002/#ps60001
(title: "applied Mounir's comments")

[commit-bot: I haz the power, 2016-10-04 11:15:14]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-04 11:20:45]

Description was changed from

==========
Hide overflow menu/closed caption list when clicking outside the list

BUG=651431
==========

to

==========
Hide overflow menu/closed caption list when clicking outside the list

BUG=651431
==========

[commit-bot: I haz the power, 2016-10-04 11:20:47]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-10-04 11:22:55]

Description was changed from

==========
Hide overflow menu/closed caption list when clicking outside the list

BUG=651431
==========

to

==========
Hide overflow menu/closed caption list when clicking outside the list

BUG=651431

Committed: https://crrev.com/2569ec9714c71a31f0135f55913a5e40c493e161
Cr-Commit-Position: refs/heads/master@{#422752}
==========

[commit-bot: I haz the power, 2016-10-04 11:22:56]

Patchset 4 (id:??) landed as
https://crrev.com/2569ec9714c71a31f0135f55913a5e40c493e161
Cr-Commit-Position: refs/heads/master@{#422752}