Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
892963003:
Ensure media control goes to transparent(hide) after seek by touch (Closed)
|
Created:
5 years, 10 months ago by william.xie1 Modified:
5 years, 10 months ago Base URL:
https://chromium.googlesource.com/chromium/blink.git@master Target Ref:
refs/heads/master Project:
blink Visibility:
Public. |
DescriptionEnsure media control goes to transparent(hide) after seek by touch
During full screen video playback, if user seek randomly
and continuously using touch screen, media control bar
will not go to transparent(hide) due to media control is
hovered and m_wasLastEventTouch is overwritten by the
following events before hideMediaControlsTimerFired, thus
IgnoreControlsHover flag is not set in
hideMediaControlsTimerFired.
This CL is target to fix this issue by setting
IgnoreControlsHover flag in startHideMediaControlsTimer
instead of hideMediaControlsTimerFired.
It doesn't impact the mouse over event to show the media
control bar persistently.
BUG=454304
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=189690
Patch Set 1 #
Total comments: 4
Patch Set 2 : Schedule the hover state update in playbackProgressed only if media control bar is opaque #
Total comments: 1
Patch Set 3 : Schedule hover state update after m_wasLastEventTouch is true #
Total comments: 1
Patch Set 4 : Ensure IgnoreControlsHover flag is set effectively #
Total comments: 1
Patch Set 5 : #
Total comments: 9
Patch Set 6 : #
Total comments: 8
Patch Set 7 #Patch Set 8 : #
Total comments: 3
Patch Set 9 : #
Total comments: 3
Patch Set 10 : #
Messages
Total messages: 40 (10 generated)
william.xie@intel.com changed reviewers: + fs@opera.com, haraken@chromium.org, philipj@opera.com
PTAL
I get the feeling that this should rather be part of the other touch-specific code (like m_wasLastEventTouch). Also, would it be possible to write a test for this? https://codereview.chromium.org/892963003/diff/1/Source/core/html/shadow/Medi... File Source/core/html/shadow/MediaControls.cpp (right): https://codereview.chromium.org/892963003/diff/1/Source/core/html/shadow/Medi... Source/core/html/shadow/MediaControls.cpp:246: document().frame()->eventHandler().scheduleHoverStateUpdate(); All this does is to start a timer, so this doesn't seem to do what the comment says. https://codereview.chromium.org/892963003/diff/1/Source/core/html/shadow/Medi... Source/core/html/shadow/MediaControls.cpp:282: if (m_panel->isOpaque() && shouldHideMediaControls()) makeTransparent already checks this, so this just seems to make the predicate more confusing. (I guess you put it here to avoid scheduling the timer above over-and-over?)
https://codereview.chromium.org/892963003/diff/1/Source/core/html/shadow/Medi... File Source/core/html/shadow/MediaControls.cpp (right): https://codereview.chromium.org/892963003/diff/1/Source/core/html/shadow/Medi... Source/core/html/shadow/MediaControls.cpp:246: document().frame()->eventHandler().scheduleHoverStateUpdate(); On 2015/02/02 13:13:46, fs wrote: > All this does is to start a timer, so this doesn't seem to do what the comment > says. Hi fs, yes, here it schedule a time for hover status update, after the the timer fired, m_panel->hovered() will get the latest state. https://codereview.chromium.org/892963003/diff/1/Source/core/html/shadow/Medi... Source/core/html/shadow/MediaControls.cpp:282: if (m_panel->isOpaque() && shouldHideMediaControls()) On 2015/02/02 13:13:46, fs wrote: > makeTransparent already checks this, so this just seems to make the predicate > more confusing. (I guess you put it here to avoid scheduling the timer above > over-and-over?) Yes, correct, to avoid schedule the hover state update time over-and-over.
Patchset #2 (id:20001) has been deleted
Patchset #2 (id:40001) has been deleted
https://codereview.chromium.org/892963003/diff/60001/Source/core/html/shadow/... File Source/core/html/shadow/MediaControls.cpp (right): https://codereview.chromium.org/892963003/diff/60001/Source/core/html/shadow/... Source/core/html/shadow/MediaControls.cpp:278: document().frame()->eventHandler().scheduleHoverStateUpdate(); Did you consider doing something like: unsigned behaviorFlags = m_wasLastEventTouch ? IgnoreControlsHover : 0; if (shouldHideMediaControls(behaviorFlags)) ...; ? Does that fix the issue you're seeing?
This reminds me of a similar problem: https://codereview.chromium.org/552903004/ I second Fredrik's suggestion to attempt a fix in or around MediaControls::shouldHideMediaControls. A test is also needed.
On 2015/02/02 15:54:11, philipj_UTC7 wrote: > This reminds me of a similar problem: https://codereview.chromium.org/552903004/ > > I second Fredrik's suggestion to attempt a fix in or around > MediaControls::shouldHideMediaControls. > > A test is also needed. Hi Philipj, I updated the patch per your review comments and move "schedule hover state update after m_wasLastEventTouch is true", It worked well too. Would you please take a look again? For the test, what kind of test do you need? Can I submit the test in another patch?
Patchset #3 (id:80001) has been deleted
The test should use eventSender to emulate the very scenario you are having trouble with. It needs to be in this CL since otherwise we don't know that this CL actually fixes the right problem. Search for runAfterHideMediaControlsTimerFired in LayoutTests/media/ for inspiration. https://codereview.chromium.org/892963003/diff/100001/Source/core/html/shadow... File Source/core/html/shadow/MediaControls.cpp (right): https://codereview.chromium.org/892963003/diff/100001/Source/core/html/shadow... Source/core/html/shadow/MediaControls.cpp:446: document().frame()->eventHandler().scheduleHoverStateUpdate(); Does this actually work? Is the problem that the hover state was *wrong*? I thought the hover state was correct but we simply wanted to ignore it for the purpose of showing/hiding controls.
Replying to out-of-band email: On Thu, Feb 5, 2015 at 3:26 PM, Xie, William <william.xie@intel.com> wrote: > > Hi Philipj, > > Thank you for your review, answer as below: > > "Does this actually work? Is the problem that the hover state was *wrong*? I thought the hover state was correct but we simply wanted to ignore it for the purpose of showing/hiding controls." > > > The problem is: > After continuous seek using touch screen, m_panel->hovered() will return TRUE and block to hide the media control bar. > > " simply ignore it for the purpose of showing/hiding controls " is also a solution, > but if there is a mouse over on the control bar, should we continue showing the media control? I think this means that it matters how the element came to be focused, not just whether or not it is currently focused. There's already some code like this: void MediaControls::hideMediaControlsTimerFired(Timer<MediaControls>*) { if (mediaElement().togglePlayStateWillPlay()) return; unsigned behaviorFlags = IgnoreFocus | IgnoreVideoHover; if (m_wasLastEventTouch) { behaviorFlags |= IgnoreControlsHover; } if (!shouldHideMediaControls(behaviorFlags)) return; makeTransparent(); m_overlayCastButton->hide(); } Is m_wasLastEventTouch true or false in your case? If it is true it seems like it should already work, so I guess it's false.
On 2015/02/05 10:09:00, philipj_UTC7 wrote: ... > On Thu, Feb 5, 2015 at 3:26 PM, Xie, William <mailto:william.xie@intel.com> wrote: ... > Is m_wasLastEventTouch true or false in your case? If it is true it seems like > it should already work, so I guess it's false. I think there's also the possibility that the timer was never started in the first place.
On 2015/02/05 10:09:00, philipj_UTC7 wrote:
> Replying to out-of-band email:
>
> On Thu, Feb 5, 2015 at 3:26 PM, Xie, William <mailto:william.xie@intel.com>
wrote:
> >
> > Hi Philipj,
> >
> > Thank you for your review, answer as below:
> >
> > "Does this actually work? Is the problem that the hover state was *wrong*? I
> thought the hover state was correct but we simply wanted to ignore it for the
> purpose of showing/hiding controls."
> >
> >
> > The problem is:
> > After continuous seek using touch screen, m_panel->hovered() will return
TRUE
> and block to hide the media control bar.
> >
> > " simply ignore it for the purpose of showing/hiding controls " is also a
> solution,
> > but if there is a mouse over on the control bar, should we continue showing
> the media control?
>
> I think this means that it matters how the element came to be focused, not
just
> whether or not it is currently focused. There's already some code like this:
>
> void MediaControls::hideMediaControlsTimerFired(Timer<MediaControls>*)
> {
> if (mediaElement().togglePlayStateWillPlay())
> return;
>
> unsigned behaviorFlags = IgnoreFocus | IgnoreVideoHover;
> if (m_wasLastEventTouch) {
> behaviorFlags |= IgnoreControlsHover;
> }
> if (!shouldHideMediaControls(behaviorFlags))
> return;
>
> makeTransparent();
> m_overlayCastButton->hide();
> }
>
> Is m_wasLastEventTouch true or false in your case? If it is true it seems like
> it should already work, so I guess it's false.
Correct, m_wasLastEventTouch is false....
Hi Philipj and fs,
I have root caused the issue, the root cause is:
If we continually seek several times, media control will be hovered, and
m_wasLastEventTouch will be overwritten by the following other events type like
change/DOMActivate etc. before hideMediaControlsTimerFired, thus
IgnoreControlsHover flag will not be set in hideMediaControlsTimerFired.
This CL is target to fix this issue by moving IgnoreControlsHover flag
setting logic from hideMediaControlsTimerFired to defaultEventHandler for each
last touch event (when m_wasLastEventTouch is true) and works well on my local
test with good user experience.
Please be noted that this change doesn't impact the mouse over event to show
the media control bar
persistently.
PTAL agin,
thanks!
William
Patchset #4 (id:120001) has been deleted
Patchset #4 (id:140001) has been deleted
Patchset #4 (id:160001) has been deleted
https://codereview.chromium.org/892963003/diff/180001/Source/core/html/shadow... File Source/core/html/shadow/MediaControls.cpp (right): https://codereview.chromium.org/892963003/diff/180001/Source/core/html/shadow... Source/core/html/shadow/MediaControls.cpp:447: m_behaviorFlags |= IgnoreControlsHover; I think I'd prefer if we could factor this slightly differently by passing the behaviorFlags to startHideMediaControlsTimer and let it set the element state (preferably rename m_behaviorFlags so that it's more obvious that it's associated with the timer). State would then be managed by start/stop (so stop would clear the flags - as would the timer-function.)
Patchset #5 (id:200001) has been deleted
On 2015/02/06 10:32:36, fs wrote: > https://codereview.chromium.org/892963003/diff/180001/Source/core/html/shadow... > File Source/core/html/shadow/MediaControls.cpp (right): > > https://codereview.chromium.org/892963003/diff/180001/Source/core/html/shadow... > Source/core/html/shadow/MediaControls.cpp:447: m_behaviorFlags |= > IgnoreControlsHover; > I think I'd prefer if we could factor this slightly differently by passing the > behaviorFlags to startHideMediaControlsTimer and let it set the element state > (preferably rename m_behaviorFlags so that it's more obvious that it's > associated with the timer). State would then be managed by start/stop (so stop > would clear the flags - as would the timer-function.) Hi fs, I re-factored the patch per your comments, would you please take a look? Thanks --William
Your description looks like it's indented - that will probably end up looking odd in the commit message, so please fix that. https://codereview.chromium.org/892963003/diff/220001/LayoutTests/media/video... File LayoutTests/media/video-controls-auto-hide-after-seek-by-touch.html (left): https://codereview.chromium.org/892963003/diff/220001/LayoutTests/media/video... LayoutTests/media/video-controls-auto-hide-after-seek-by-touch.html:46: <p>Test video element control visibility when mouse is not over element.</p> I think keeping some for of description would be good. https://codereview.chromium.org/892963003/diff/220001/LayoutTests/media/video... File LayoutTests/media/video-controls-auto-hide-after-seek-by-touch.html (right): https://codereview.chromium.org/892963003/diff/220001/LayoutTests/media/video... LayoutTests/media/video-controls-auto-hide-after-seek-by-touch.html:39: dispatchActivateEvent(controls); Won't the activate event be generated as a side-effect of the tap? https://codereview.chromium.org/892963003/diff/220001/Source/core/html/shadow... File Source/core/html/shadow/MediaControls.cpp (right): https://codereview.chromium.org/892963003/diff/220001/Source/core/html/shadow... Source/core/html/shadow/MediaControls.cpp:482: m_HideTimerBehaviorFlags |= IgnoreVideoHover; I'd make this similar to the previous PS: unsigned behaviorFlags = m_hideTimerBehaviorFlags | ...; I.e. don't mutate the member here - just read it and pass it. Alternatively you could clear it here (first in the method) too, but if it isn't mutated it should be ok to leave that to start...(). https://codereview.chromium.org/892963003/diff/220001/Source/core/html/shadow... Source/core/html/shadow/MediaControls.cpp:492: if (m_wasLastEventTouch) { Get rid of m_wasLastEventTouch, and either pass that to startHideMediaControlsTimer: startHideMediaControlsTimer(bool triggeredByTouch) or determine the behavior flags in defaultEventHandler and pass that. startHideMediaControlsTimer(unsigned behaviorFlags) https://codereview.chromium.org/892963003/diff/220001/Source/core/html/shadow... File Source/core/html/shadow/MediaControls.h (right): https://codereview.chromium.org/892963003/diff/220001/Source/core/html/shadow... Source/core/html/shadow/MediaControls.h:141: unsigned m_HideTimerBehaviorFlags; m_hideTimer... Also, move it up to just after m_hideMediaControlsTimer.
Thank you so much for your review, fs. PTAL again. https://codereview.chromium.org/892963003/diff/220001/LayoutTests/media/video... File LayoutTests/media/video-controls-auto-hide-after-seek-by-touch.html (right): https://codereview.chromium.org/892963003/diff/220001/LayoutTests/media/video... LayoutTests/media/video-controls-auto-hide-after-seek-by-touch.html:39: dispatchActivateEvent(controls); On 2015/02/06 13:43:32, fs wrote: > Won't the activate event be generated as a side-effect of the tap? DOMActivate event is intended event from Node::dispatchDOMActivateEvent, here we simplify the reproduceable steps as much as possible. https://codereview.chromium.org/892963003/diff/220001/Source/core/html/shadow... File Source/core/html/shadow/MediaControls.cpp (right): https://codereview.chromium.org/892963003/diff/220001/Source/core/html/shadow... Source/core/html/shadow/MediaControls.cpp:482: m_HideTimerBehaviorFlags |= IgnoreVideoHover; On 2015/02/06 13:43:32, fs wrote: > I'd make this similar to the previous PS: > > unsigned behaviorFlags = m_hideTimerBehaviorFlags | ...; > > I.e. don't mutate the member here - just read it and pass it. > > Alternatively you could clear it here (first in the method) too, but if it isn't > mutated it should be ok to leave that to start...(). Done.
https://codereview.chromium.org/892963003/diff/220001/LayoutTests/media/video... File LayoutTests/media/video-controls-auto-hide-after-seek-by-touch.html (right): https://codereview.chromium.org/892963003/diff/220001/LayoutTests/media/video... LayoutTests/media/video-controls-auto-hide-after-seek-by-touch.html:20: target.dispatchEvent(event); Inconsistent indentation. https://codereview.chromium.org/892963003/diff/220001/LayoutTests/media/video... LayoutTests/media/video-controls-auto-hide-after-seek-by-touch.html:39: dispatchActivateEvent(controls); On 2015/02/06 14:59:20, william.xie wrote: > On 2015/02/06 13:43:32, fs wrote: > > Won't the activate event be generated as a side-effect of the tap? > > DOMActivate event is intended event from Node::dispatchDOMActivateEvent, here we > simplify the reproduceable steps as much as possible. Yes, but if it will be generated anyway, then it doesn't seem to really "simplify" the test... https://codereview.chromium.org/892963003/diff/240001/LayoutTests/media/video... File LayoutTests/media/video-controls-auto-hide-after-seek-by-touch.html (right): https://codereview.chromium.org/892963003/diff/240001/LayoutTests/media/video... LayoutTests/media/video-controls-auto-hide-after-seek-by-touch.html:53: <p>Test video control element visibility when seek by touch.</p> It's actually pressing 'play' so s/seek/play/ https://codereview.chromium.org/892963003/diff/240001/Source/core/html/shadow... File Source/core/html/shadow/MediaControls.cpp (right): https://codereview.chromium.org/892963003/diff/240001/Source/core/html/shadow... Source/core/html/shadow/MediaControls.cpp:445: startHideMediaControlsTimer(IgnoreControlsHover); unsigned hideBehavior = wasLastEventTouch ? IgnoreControlsHover : IgnoreNone; and then pass that as appropriate to the invocations of startHideMediaControlsTimer(). If that works... because we're obviously not checking for the same types of events as below... that could mean that we need to keep m_wasLastEventTouch. https://codereview.chromium.org/892963003/diff/240001/Source/core/html/shadow... Source/core/html/shadow/MediaControls.cpp:494: m_hideTimerBehaviorFlags |= behaviorFlags; This ought to be a simple assignment.
Dear fs, I updated the layout test per your comments and answered the questions about changes in MediaControls.cpp, Could you kindly take a look again? William https://codereview.chromium.org/892963003/diff/240001/LayoutTests/media/video... File LayoutTests/media/video-controls-auto-hide-after-seek-by-touch.html (right): https://codereview.chromium.org/892963003/diff/240001/LayoutTests/media/video... LayoutTests/media/video-controls-auto-hide-after-seek-by-touch.html:53: <p>Test video control element visibility when seek by touch.</p> On 2015/02/06 15:20:59, fs wrote: > It's actually pressing 'play' so s/seek/play/ Done. https://codereview.chromium.org/892963003/diff/240001/Source/core/html/shadow... File Source/core/html/shadow/MediaControls.cpp (right): https://codereview.chromium.org/892963003/diff/240001/Source/core/html/shadow... Source/core/html/shadow/MediaControls.cpp:445: startHideMediaControlsTimer(IgnoreControlsHover); On 2015/02/06 15:20:59, fs wrote: > unsigned hideBehavior = wasLastEventTouch ? IgnoreControlsHover : IgnoreNone; > > and then pass that as appropriate to the invocations of > startHideMediaControlsTimer(). If that works... because we're obviously not > checking for the same types of events as below... that could mean that we need > to keep m_wasLastEventTouch. Currently we need to invoke startHideMediaControlsTimer here to mark the IgnoreControlsHover... https://codereview.chromium.org/892963003/diff/240001/Source/core/html/shadow... Source/core/html/shadow/MediaControls.cpp:494: m_hideTimerBehaviorFlags |= behaviorFlags; On 2015/02/06 15:20:59, fs wrote: > This ought to be a simple assignment. You know, startHideMediaControlsTimer is also called by MediaControls::playbackStarted() if we use simple assignment, the IgnoreControlsHover flag will be overwritten before hideMediaControlsTimerFired, thus cannot hide the control bar....
Your description now lack the subject line. Re-add the issue title: Ensure media control goes to transparent(hide) after seek by touch as the first line with a blank line following it. https://codereview.chromium.org/892963003/diff/240001/Source/core/html/shadow... File Source/core/html/shadow/MediaControls.cpp (right): https://codereview.chromium.org/892963003/diff/240001/Source/core/html/shadow... Source/core/html/shadow/MediaControls.cpp:445: startHideMediaControlsTimer(IgnoreControlsHover); On 2015/02/06 15:45:10, william.xie wrote: > On 2015/02/06 15:20:59, fs wrote: > > unsigned hideBehavior = wasLastEventTouch ? IgnoreControlsHover : IgnoreNone; > > > > and then pass that as appropriate to the invocations of > > startHideMediaControlsTimer(). If that works... because we're obviously not > > checking for the same types of events as below... that could mean that we need > > to keep m_wasLastEventTouch. > > Currently we need to invoke startHideMediaControlsTimer here to mark the > IgnoreControlsHover... This could end up starting the timer regardless of whether the event was handled or not (=> the timer was started). Although I don't really like it maybe we need a new method that accumulates flags ("configures the next hide timer"), even though I don't particularly like that since it makes the system very hard to reason about. It sort of matches the current behavior though I guess... https://codereview.chromium.org/892963003/diff/240001/Source/core/html/shadow... Source/core/html/shadow/MediaControls.cpp:494: m_hideTimerBehaviorFlags |= behaviorFlags; On 2015/02/06 15:45:10, william.xie wrote: > On 2015/02/06 15:20:59, fs wrote: > > This ought to be a simple assignment. > > You know, startHideMediaControlsTimer is also called by > MediaControls::playbackStarted() > if we use simple assignment, the IgnoreControlsHover flag will be overwritten > before hideMediaControlsTimerFired, thus cannot hide the control bar.... Ah ok. But that means that the stored flags need to be cleared when the timer fires (otherwise only an intervening stop() of the timer would clear the flags).
Dear fs, Make a much simpler one. Would you please take a look it? William
Hi Philipj, Would you please take a look? William
https://codereview.chromium.org/892963003/diff/280001/Source/core/html/shadow... File Source/core/html/shadow/MediaControls.cpp (right): https://codereview.chromium.org/892963003/diff/280001/Source/core/html/shadow... Source/core/html/shadow/MediaControls.cpp:445: m_hideTimerBehaviorFlags |= m_wasLastEventTouch ? IgnoreControlsHover : IgnoreNone; Please add a comment describing why we need to do this here. https://codereview.chromium.org/892963003/diff/280001/Source/core/html/shadow... Source/core/html/shadow/MediaControls.cpp:489: m_hideTimerBehaviorFlags = IgnoreNone; This should always be done, so should go at the top. Something like: unsigned behaviorFlags = m_hideTimerBehaviorFlags; m_hideTimerBehaviorFlags = IgnoreNone; if (mediaElement().togglePlayStateWillPlay()) return; <the rest of the function>
I'm currently looking at a similar issue where I want to show the video controls on any touch and *if* I've done so do nothing in the default event handler of the synthetic click event that would follow. Not directly related to the problem, but really it's about behaving differently if an event was triggered by a synthetic event due to touch events. (Aside: I just don't understand how Web developers are able to deal with this mess.) It would be great if we could make explicit the ideas of "this element is hovered because of a touch event" or "this element is focused because of a mouse click" and treat those specially. I'm a little bit concerned about adding state bits like m_hideTimerBehaviorFlags, seems easy to get that into the wrong state if the order of events isn't as expected. Sorry for the rambling and lack of constructive feedback, I will let fs take a look at PS8 to see how he likes it.
Oops, comments out-of-order :)
Hi fs, Would you please take a look again? William https://codereview.chromium.org/892963003/diff/280001/Source/core/html/shadow... File Source/core/html/shadow/MediaControls.cpp (right): https://codereview.chromium.org/892963003/diff/280001/Source/core/html/shadow... Source/core/html/shadow/MediaControls.cpp:445: m_hideTimerBehaviorFlags |= m_wasLastEventTouch ? IgnoreControlsHover : IgnoreNone; On 2015/02/06 16:28:58, fs wrote: > Please add a comment describing why we need to do this here. Done.
On 2015/02/06 16:34:40, philipj_UTC7 wrote: > Oops, comments out-of-order :) Sorry philip, it's my fault, Have a nice day! William
LGTM w/ nits https://codereview.chromium.org/892963003/diff/300001/LayoutTests/media/video... File LayoutTests/media/video-controls-auto-hide-after-play-by-touch.html (right): https://codereview.chromium.org/892963003/diff/300001/LayoutTests/media/video... LayoutTests/media/video-controls-auto-hide-after-play-by-touch.html:20: target.dispatchEvent(event); Nit: Align/indent to same column as the two lines above. https://codereview.chromium.org/892963003/diff/300001/Source/core/html/shadow... File Source/core/html/shadow/MediaControls.cpp (right): https://codereview.chromium.org/892963003/diff/300001/Source/core/html/shadow... Source/core/html/shadow/MediaControls.cpp:445: // We mark IgnoreControlsHover to m_hideTimerBehaviorFlags for last touch event Nit: Maybe reword this a bit: Add IgnoreControlsHover to m_hideTimerBehaviorFlags when we see a touch event, to allow the hide-timer to do the right thing when it fires. FIXME: Preferably we would only do this when we're actually handling the event here ourselves.
New patchsets have been uploaded after l-g-t-m from fs@opera.com
Fixed the nits, Thank you so much for your great review, fs... Have a nice weekend.. William https://codereview.chromium.org/892963003/diff/300001/Source/core/html/shadow... File Source/core/html/shadow/MediaControls.cpp (right): https://codereview.chromium.org/892963003/diff/300001/Source/core/html/shadow... Source/core/html/shadow/MediaControls.cpp:445: // We mark IgnoreControlsHover to m_hideTimerBehaviorFlags for last touch event On 2015/02/06 17:04:37, fs wrote: > Nit: Maybe reword this a bit: > > Add IgnoreControlsHover to m_hideTimerBehaviorFlags when we see a touch event, > to allow the hide-timer to do the right thing when it fires. > FIXME: Preferably we would only do this when we're actually handling the event > here ourselves. Done.
The CQ bit was checked by william.xie@intel.com
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/892963003/320001
Message was sent while issue was closed.
Committed patchset #10 (id:320001) as https://src.chromium.org/viewvc/blink?view=rev&revision=189690 |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
