If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

Video doesn't seek if page prevented mousedown event or mouseup event

ASSIGNED
Assigned to

Status

()

Toolkit
Video/Audio Controls
ASSIGNED
9 months ago
6 months ago

People

(Reporter: arni2033, Assigned: timdream)

Tracking

({regression})

53 Branch
regression
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox50 unaffected, firefox51 unaffected, firefox52 unaffected)

Details

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

9 months ago
>>>   My Info:   Win7_64, Nightly 53, 32bit, ID 20161119030204 (2016-11-19)
STR_1:  (testcase)
1. Open https://www.iandevlin.com/html5test/webvtt/html5-video-webvtt-sample.html
2. Add attribute [onmousedown="return false"] to <body>
3. Click on the middle of timeline in video controls

AR:  No visible action
ER:  Video should seek to the middle in a normal way, just like on GoogleChrome


STR_2:  (original)
1. Open any video in comment on funnyjunk.com
2. Click on the middle of timeline in video controls

AR:  No visible action
ER:  Video should seek to the middle, just like on GoogleChrome


STR_3:  (testcase)
1. Open https://www.iandevlin.com/html5test/webvtt/html5-video-webvtt-sample.html
2. Add attribute [onmouseup="return false"] to <body>
3. Click on the middle of timeline in video controls
4. Move mouse a bit

AR:
 Step 3 - video seeks to the middle.
 Step 4 - video goes back to the same place in timeline where it was before Step 3
ER:
 Video should seek to the middle in a normal way, just like on GoogleChrome
(Reporter)

Updated

9 months ago
Component: Untriaged → Video/Audio Controls
Product: Firefox → Toolkit
(Reporter)

Updated

9 months ago
No longer blocks: 1277113
I have managed to reproduce this issue following the steps from STR_1, below is the result:

Narrowed inbound regression window from [28e2a6dd, 6186126f] (3 revisions) to [46127b3a, 6186126f] (2 revisions) (~1 steps left)
Oh noes, no (more) inbound revisions :(
Last good revision: 46127b3a981bceb0413c8199849f4e47afc949da
First bad revision: 6186126f502ba47e4fb2b6f4d971ea6fd3e66a02
Pushlog:
https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=46127b3a981bceb0413c8199849f4e47afc949da&tochange=6186126f502ba47e4fb2b6f4d971ea6fd3e66a02
status-firefox50: --- → unaffected
status-firefox51: --- → unaffected
status-firefox52: --- → unaffected
status-firefox53: --- → affected
Keywords: regressionwindow-wanted
Version: Trunk → 53 Branch

Comment 2

9 months ago
I did some tests on slider(input[type="range"]) and noticed that not only the slider inside video control is broken, but actually all input[type="range"] are affected. As we converted all xul element to HTML in bug 1271765, it is expectable that slider suffered from the same issue.

Jared, do you know who are the right person to this problem? Thanks :)
Flags: needinfo?(jaws)
Redirecting to :stone as that is who helped me with bug 1295719.
Flags: needinfo?(jaws) → needinfo?(sshih)

Updated

9 months ago
Assignee: nobody → sshih
Flags: needinfo?(sshih)
Return false in mousedown's listener stops the default behavior [1]. That stops updating the value of range element and no input event is fired. The listener in [2] is not triggered either.

Tested on Edge Microsoft Edge 38.14393.0.0
  return false doesn't stop dragging and seeking video.
  call preventDefault stops dragging and seeking video.
Tested on Chrome 57.0.2975.0 
  return false doesn't stop dragging and seeking video.
  call preventDefault doesn't stop dragging and seeking video.


[1] http://searchfox.org/mozilla-central/rev/0f254a30d684796bcc8b6e2a102a0095d25842bb/dom/events/JSEventHandler.cpp#224
[2] https://hg.mozilla.org/mozilla-central/rev/083604641e50#l2.1090
Corrected testing results on Edge
Tested on Microsoft Edge 38.14393.0.0
  UA fires mousedown event to body element when clicking on the video control
  Added attribute onmousedown="return false" to body element stops dragging and seeking video.
  call preventDefault stops dragging and seeking video.

Tested on Safari 9.1.1 (11601.6.17)
  behaves the same as Edge

Tested on Firefox 53.0a1 (2017-01-09) (64-bit)
  behaves the same as Edge

Tested on Chrome 57.0.2975.0 
  UA doesn't fire mousedown event to body element when clicking on the video control
  Added attribute onmousedown="return false" to body element doesn't stop dragging and seeking video.
  call preventDefault doesn't stop dragging and seeking video.
Tested with audio control (http://www.html5tutorial.info/html5-audio.php)
Tested on Microsoft Edge 38.14393.0.0
  Click on audio progress bar fires mousedown
    Added attribute onmousedown="return false" stops dragging and seeking audio progress.
    Call preventDefault stops dragging and seeking audio progress.
  Click on audio volume bar fires mousedown
    Added attribute onmousedown="return false" stops dragging audio volume bar.
    Call preventDefault stops dragging audio volume bar.

Tested on Safari 9.1.1 (11601.6.17)
  No audio progress control
  Audio volume behaves the same as Edge

Tested on Firefox 53.0a1 (2017-01-09) (64-bit)
  Doesn't fire mousedown
  Drags audio progress and volume bar

Tested on Chrome 57.0.2975.0
  Behaves the same as Firefox
Tested with <select> tag (http://examples.quackit.com/preview/html_editor_result.cfm?contentFile=/html_5/tags/_inc/inc_html_select_tag.cfm)
Tested on Microsoft Edge 38.14393.0.0
  Fires mousedown
  PreventDefault or add attribute onmousedown="return false" stop its behavior

Tested on Safari 9.1.1 (11601.6.17)
  Behaves the same as Edge

Tested on Firefox 53.0a1 (2017-01-09) (64-bit)
  Behaves the same as Edge

Tested on Chrome 57.0.2975.0
  Behaves the same as Edge
In summary
         Video Control  Audio Control     Select
Edge     1              1                 1
Safari   1              1                 1
Firefox  1              2                 1
Chrome   2              2                 1

1: fires mousedown event, prevent default or add attribute onmousedown="return false" stops its action
2: doesn't fire mousedown event, prevent default and add attribute onmousedown "return false" doesn't stop its action

Comment 9

8 months ago
What you mean with 2, "doesn't fire mousedown event"? Do you not get event at all, or not in bubble phase or what?

Comment 10

8 months ago
FWIW, http://searchfox.org/mozilla-central/rev/225ab0637ed51b8b3f9f4ee2f9c339a37a65b626/toolkit/content/widgets/videocontrols.xml#1454-1455 is rather suspicious looking code. Why is that calling stopPropagation.

Comment 11

8 months ago
That particular code would be consistent with the rest of the videocontrol handling if
 { mozSystemGroup: true } was passed to addEventListener as 3rd param.

Comment 12

8 months ago
https://github.com/whatwg/html/issues/2258
The audio testing results are updated because I misused a website with iframe.
         Video Control  Audio Control     Select
Edge     1              1                 1
Safari   1              1                 1
Firefox  1              1                 1
Chrome   2              2                 1

1: fires mousedown event, prevent default or add attribute onmousedown="return false" stops its action
2: doesn't fire mousedown event (checked via listeners in capturing and bubbling phase), prevent default and add attribute onmousedown "return false" doesn't stop its action

Note:
Edge stops the actions of all controls when click on it with preventDefault is called.
Safari only stops progress seeking and volume adjustment. Play/suspend and mute/unmute is still working.
Firefox only stops progress seeking and volume adjustment. Play/suspend and mute/unmute is still working.
On Firefox, preventDefault on click can stop play/resume control's behavior. Prevent default on mousedown, mouseup, click doesn't stop mute/unmute control's behavior.

Comment 15

8 months ago
Thanks for looking into this bug, :stone.

As we discussed offline, another slider issue(bug 1327238) might related to input[type="range"] as well.

Comment 16

8 months ago
(In reply to Olli Pettay [:smaug] from comment #10)
> FWIW,
> http://searchfox.org/mozilla-central/rev/
> 225ab0637ed51b8b3f9f4ee2f9c339a37a65b626/toolkit/content/widgets/
> videocontrols.xml#1454-1455 is rather suspicious looking code. Why is that
> calling stopPropagation.

The stopPropagation is for texttrack button which overlay video content area. It's intended to avoid triggering video play/pause while the button being selecting.

Comment 17

8 months ago
Ray, could the listener use { mozSystemGroup: true } and then possibly preventDefault()?
{ mozSystemGroup: true } makes this not visible to the web content.

Comment 18

8 months ago
Hi Olli,

I think the listener of slider uses mozSystemGroup already[0]. IIRC, mozSystemGroup has its own procedure rather than web content event, so I had a hard time figure out the reason how slider being affected. 

Let me try preventDefault(), and I'll update the result soon later :) Thanks.


[0] https://dxr.mozilla.org/mozilla-central/rev/13603af3862d9583ed2feefb06e0988c2d7fed8c/toolkit/content/widgets/videocontrols.xml#1720-1724,1746-1747

Comment 19

8 months ago
The relevant code I linked doesn't use mozSystemGroup, but it probably should.

Comment 20

8 months ago
Sorry for the late reply.

yeh, all event listeners in anonymous content should use `mozSystemGroup`, thanks.

----
However, the problem is still, and I wonder is this behavior happened on typical websites or not? It seems likely a sort of bad intention by website author, and I don't see an urgent necessary to fix(support) it right away. Also, per survey by :stone, our implementation is aligned with Safari and even a tiny bit better than Edge. Should we consider removing this bug from blocker list of bug 1271765?

Hani, what do you think about this? Thanks.
Flags: needinfo?(hani.yacoub)

Comment 21

8 months ago
I think it's OK not to be fixed in Nightly and focus on the bugs that have higher impact on this feature, but this bug should be fixed in aurora.
Flags: needinfo?(hani.yacoub)
If you land a fix after next week's merge, could you be sure to request uplift to aurora? Thanks.
tracking-firefox53: --- → +
(In reply to Liz Henry (:lizzard) (needinfo? me) from comment #22)
> If you land a fix after next week's merge, could you be sure to request
> uplift to aurora? Thanks.

Sorry for replying late. We behave the same as Edge and Safari. I think it's hard to say current behavior is incorrect at this moment. There is a spec issue fired in [1] to be discussed but I have no idea sure whether we can get the conclusion soon. Please ni me if need other actions or information. Thanks.

[1] https://github.com/whatwg/html/issues/2258
Flags: needinfo?(lhenry)
OK. Thanks!  Hani, by "should be fixed in aurora" do you mean it is fixed already in aurora 52, or that it still needs to be fixed in 53 once 53 moves to aurora? I realize on second look that your meaning is unclear.
Flags: needinfo?(lhenry) → needinfo?(hani.yacoub)

Comment 25

8 months ago
What I meant was that it's still need to be fix in Firefox Nightly 54 and then uplifted to Aurora 53.
Flags: needinfo?(hani.yacoub)
Any word on a fix here, or a resolution of the spec issue?
Flags: needinfo?(sshih)
This is still considered as a bug after discussions with ralin, timdream, and chsiang. This bug will be handover to ralin and he plans to create another bug to request the flexibility to prevent the default behavior of anonymous elements be impacted by the content. Maybe ralin could update more details about next action.
Flags: needinfo?(sshih) → needinfo?(ralin)

Updated

7 months ago
Depends on: 1338961

Comment 28

7 months ago
After lengthy offline discussion, we have a proposal to fix this in bug 1338961. This bug has nothing to do with mozSystemGroup, just to be clear.
Flags: needinfo?(ralin)
We will need to look at bug 1338961 to see if we could get this fixed in fx53. It's unfortunate, but we will not be able to fix this in JS without that feature.

Without waiting for a resolution from the spec, the assumption in Fx Front-end and Product is that we should try to keep the original behavior (i.e. fix this regression) whatever the underlining markup we are using to construct the UI.
Assignee: sshih → ralin
Status: NEW → ASSIGNED
tracking-firefox53: + → ?
So while discussing bug 1338961, I checked the Chrome video control implementation. Their video control is hosted inside a user agent shadow DOM. It seems that they avoid this bug by never dispatch the event interacting with video controls (<div pseudo="-webkit-media-controls-enclosure">) to the document. No-op mousedown events on the non-control part of the video element does get dispatched, though.

Our event dispatcher does not allow XBL script to do that, since system event group receives the event *after* capturing and bubbling phase ( http://searchfox.org/mozilla-central/rev/ac8a72f2d5204ced2004c93a9c193430c63a6a71/dom/events/EventDispatcher.cpp#419-529 ). That's something we could consider supporting in order not to implement the hack in bug 1338961.

Comment 31

7 months ago
Chrome's behavior sounds like a bug to me. Why is it doing that?
Rick, perhaps you know, or you know who to ask?
Flags: needinfo?(rbyers)

Comment 32

7 months ago
You don't even see the mousedown event on the document in the capture phase?  That's not what I'm seeing in a quick test on youtube.com - mousedown on the slider and I see the mousedown reach the document both during capture and bubbling phases.

Regardless mlamouri@chromium.org is the expert, and he's told me our DOM events system really should adopt Gecko's mechanism for dealing with internal controls (though I can't find that thread right now - was there a bug?).
Flags: needinfo?(rbyers) → needinfo?(mounir)
(In reply to Rick Byers from comment #32)
> You don't even see the mousedown event on the document in the capture phase?
> That's not what I'm seeing in a quick test on youtube.com - mousedown on the
> slider and I see the mousedown reach the document both during capture and
> bubbling phases.

We are talking about native video control here. YouTube implements it's own control so you won't see the behavior here.

There is my test:

1. Go to http://camendesign.com/code/video_for_everybody/test.html
2. Open up DevTools, inspect the <body> element and do the following

$0.addEventListener('mousedown', (evt) => console.log(evt, 'capture'), true)
$0.addEventListener('mousedown', (evt) => console.log(evt, 'bubble'))

3. Click on the video control (play/pause button, or seeker, anything).

You can verify DevTools works by click on other part of the page, or the video itself.
(In reply to Rick Byers from comment #32)
> You don't even see the mousedown event on the document in the capture phase?
> That's not what I'm seeing in a quick test on youtube.com - mousedown on the
> slider and I see the mousedown reach the document both during capture and
> bubbling phases.
> 
> Regardless mlamouri@chromium.org is the expert, and he's told me our DOM
> events system really should adopt Gecko's mechanism for dealing with
> internal controls (though I can't find that thread right now - was there a
> bug?).

I'm not sure who is that mlamouri@chromium.org but I'm no expert :)

Media controls in Blink are a nice mess and because we don't have a system group like you have in Gecko, we do weird things in order to make sure we receive the events. In this case, I think the patch that created this weird behaviour is https://codereview.chromium.org/406213002 which is linked to these two bugs: https://bugs.chromium.org/p/chromium/issues/detail?id=388738 https://bugs.chromium.org/p/chromium/issues/detail?id=269454

It seems that one of the reasons why this change was created is https://github.com/whatwg/html/commit/ef57900202d092434b3e4bfef29481e7767d4a1f which suggests that UA should not propagate events from the native controls to the page. At least, that's my understand.
Flags: needinfo?(mounir)

Comment 35

7 months ago
wow, that spec is just buggy, super vague. Is mousemove user interaction event? 

Anyhow, if we want to mimic not-dispatching-events-at-all, we could easily just stop propagation in
to <video> and <audio> by having
HTMLMediaElement::GetEventTargetParent method and set
aVisitor.mCanHandle = false;  there.
Probably need to do that only for some events (which ones? The spec is totally unclear here), and only trusted events, and only when "native" controls are used.

Comment 36

7 months ago
https://github.com/whatwg/html/issues/2370
Comment hidden (obsolete)
I can try to deliver a fix proposed in comment 35, but probably not in the fx53 cycle. Let's land the localized hack in bug 1338961 first...
Assignee: ralin → timdream
status-firefox53: affected → ---
tracking-firefox53: ? → ---
Comment hidden (obsolete)
Comment hidden (obsolete)
.(In reply to Tim Guan-tin Chien [:timdream] (please needinfo) from comment #37)
> nit: not the entire <video> or <audio> element, just the video control
> within it. It would be a bit hard to figure out that part of the UI in
> native code (the binding host <xul:videocontrol> also occupied the full
> video area).

... and this is not correct because Firefox do respond to mouse click on non-control area of the <video> frame.

This means by definition of the spec, the "user interface exposed to the user" occupied the entire <video> frame and we must not fire any events of mouse/touch/keyboard from it. I wonder if this is acceptable...
Note that Blink is looking into implementing keyboard controls on the video element and that will make the problem similar to what you have in Gecko.
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
My current patch implements what's said in comment 35 but disregards event dispatching of keyboard event. The same approach won't work with keyboard event because the binding also listens for them on the <video> itself -- setting mCanHandle on the element will break the video control too. I think this is right balance because this is how Firefox used to work with XUL video control (allow content to listen to events but ignore it's defaultPrevented flag).

Obviously spec can be more clearer about key events (see comment 42) and we can address that on another bug when it does.
Comment hidden (mozreview-request)

Comment 48

6 months ago
mozreview-review
Comment on attachment 8846508 [details]
Bug 1327097 - Part III, Allow a11y test harness to listen event on the DOM element,

https://reviewboard.mozilla.org/r/119570/#review121484

I guess the other patch explains why we can revert this :)
Attachment #8846508 - Flags: review?(bugs) → review+

Comment 49

6 months ago
mozreview-review
Comment on attachment 8846509 [details]
Bug 1327097 - Part II, Trap mouse/touch/pointer events in audio/video element,

https://reviewboard.mozilla.org/r/119572/#review121486

::: dom/html/HTMLMediaElement.cpp:4078
(Diff revision 3)
>  }
>  
> +nsresult
> +HTMLMediaElement::GetEventTargetParent(EventChainPreVisitor& aVisitor)
> +{
> +  RefPtr<HTMLMediaElement> self(this);

Why do you need this RefPtr? It just slows down this possibly hot code.

::: dom/html/HTMLMediaElement.cpp:4081
(Diff revision 3)
> +HTMLMediaElement::GetEventTargetParent(EventChainPreVisitor& aVisitor)
> +{
> +  RefPtr<HTMLMediaElement> self(this);
> +
> +  if (!self->Controls()) {
> +    aVisitor.mCanHandle = true;

No need to set mCanHandle = true, since the parent classes will do that.

::: dom/html/HTMLMediaElement.cpp:4086
(Diff revision 3)
> +    aVisitor.mCanHandle = true;
> +    return nsGenericHTMLElement::GetEventTargetParent(aVisitor);
> +  }
> +
> +  if (!aVisitor.mEvent->mFlags.mIsTrusted) {
> +    aVisitor.mCanHandle = true;

ditto.

And I would just merge the two ifs.

if (!Controls() || !aVisitor.mEvent->mFlags.mIsTrusted) {
  return return nsGenericHTMLElement::GetEventTargetParent(aVisitor);
}

::: dom/html/HTMLMediaElement.cpp:4092
(Diff revision 3)
> +    return nsGenericHTMLElement::GetEventTargetParent(aVisitor);
> +  }
> +
> +  // We will need to trap mouse events within the media element,
> +  // preventing the content from handling them.
> +  switch (aVisitor.mEvent->mMessage) {

Do you need to handle also pointer events here?
What does blink do with those?
It would be odd to get pointerdown/up, but not mousedown/up.
What about touch events?

What happens if one does mousedown on some other element and then mouseup on the video?
How does blink behave then?
(you may need to call preventDefault on the mousedown or mousemove to prevent drag session to be initialized)

::: dom/html/HTMLMediaElement.cpp:4100
(Diff revision 3)
> +    case eMouseDoubleClick:
> +    case eMouseUp:
> +      aVisitor.mCanHandle = false;
> +      return NS_OK;
> +    default:
> +      aVisitor.mCanHandle = true;

No need to set mCanHandle to true
Attachment #8846509 - Flags: review?(bugs) → review-
(In reply to Olli Pettay [:smaug] from comment #49)
> ::: dom/html/HTMLMediaElement.cpp:4092
> (Diff revision 3)
> > +    return nsGenericHTMLElement::GetEventTargetParent(aVisitor);
> > +  }
> > +
> > +  // We will need to trap mouse events within the media element,
> > +  // preventing the content from handling them.
> > +  switch (aVisitor.mEvent->mMessage) {
> 
> Do you need to handle also pointer events here?
> What does blink do with those?
> It would be odd to get pointerdown/up, but not mousedown/up.
> What about touch events?
> 

Blink contains touch, keyboard, and mouse events and does not handle pointer events:

https://codereview.chromium.org/406213002/patch/120001/130009
https://cs.chromium.org/chromium/src/third_party/WebKit/Source/core/html/shadow/MediaControlElements.cpp?l=80&rcl=a15fea4b2a0062f98fa2c7b446dfa4035e7231c9

Does call preventDefault on pointer event also prevent the touch/mouse event from firing? Will test tomorrow.

> What happens if one does mousedown on some other element and then mouseup on
> the video?
> How does blink behave then?
> (you may need to call preventDefault on the mousedown or mousemove to
> prevent drag session to be initialized)

Interesting. Will test this out tomorrow too.

Comment 51

6 months ago
The code for slider seems to dealing with pointerevents these days.
Hi Yura,

Quick question for you. My patch here will contain click/mouse/touch/pointer events into the <video> element itself. The change breaks ally test on media controls [1] because it binds the event listener on the outer document [2], not the element itself.

[1] https://treeherder.mozilla.org/#/jobs?repo=try&author=timdream@gmail.com&selectedJob=83338794
[2] http://searchfox.org/mozilla-central/rev/a5c2b278897272497e14a8481513fee34bbc7e2c/accessible/tests/mochitest/events.js#782

Can I change the object in eventSeq (created by checkerOfActionInvoker()) such that events.js will bind the listener on the DOM node itself on this case? I will ask you for review for that. Thanks.
Flags: needinfo?(yzenevich)
Comment hidden (mozreview-request)
(Assignee)

Comment 54

6 months ago
mozreview-review-reply
Comment on attachment 8846509 [details]
Bug 1327097 - Part II, Trap mouse/touch/pointer events in audio/video element,

https://reviewboard.mozilla.org/r/119572/#review121486

> Do you need to handle also pointer events here?
> What does blink do with those?
> It would be odd to get pointerdown/up, but not mousedown/up.
> What about touch events?
> 
> What happens if one does mousedown on some other element and then mouseup on the video?
> How does blink behave then?
> (you may need to call preventDefault on the mousedown or mousemove to prevent drag session to be initialized)

The new patch now handles pointer & touch events, because they can also be used to cancel slider drag per my tests.

If the user does mousedown on some other element and then mouseup on the video, the document will only hear mousedown. This is consistent with what Blink is doing and just like if the user mousedown on the document and mouseup outside of the window.

Comment 55

6 months ago
Blink's behavior is horrible. Can we do anything better.

Mounir, you might know why blink is doing what it is doing. Why it even initially started to have that not-compatible-with-the-rest-of-the-DOM behavior?
Flags: needinfo?(mounir)
Not sure what the question is exactly. If this is the general behaviour, I believe I gave some links above and that's the only thing I know. I can dig deeper if it can help. If the question is about the pointer events, it's a bug and we should handle them.

Feel free to ping me on IRC if it can help.
Flags: needinfo?(mounir)

Comment 57

6 months ago
Blink doesn't seem to trap mousemoves when over video, if one isn't dragging some slider.

I think we definitely want at least that behavior, so that not all mousemoves are broken on top of video.

Comment 58

6 months ago
mozreview-review
Comment on attachment 8846509 [details]
Bug 1327097 - Part II, Trap mouse/touch/pointer events in audio/video element,

https://reviewboard.mozilla.org/r/119572/#review121996

See the previous comment in bugzilla.
I think we need different handling for *move events.

Also, does blink trap all these events even when on top of the video area, or only when on top of control area?
Attachment #8846509 - Flags: review?(bugs) → review-
(In reply to Tim Guan-tin Chien [:timdream] (please needinfo) from comment #52)
> Hi Yura,
> 
> Quick question for you. My patch here will contain click/mouse/touch/pointer
> events into the <video> element itself. The change breaks ally test on media
> controls [1] because it binds the event listener on the outer document [2],
> not the element itself.
> 
> [1]
> https://treeherder.mozilla.org/#/jobs?repo=try&author=timdream@gmail.
> com&selectedJob=83338794
> [2]
> http://searchfox.org/mozilla-central/rev/
> a5c2b278897272497e14a8481513fee34bbc7e2c/accessible/tests/mochitest/events.
> js#782
> 
> Can I change the object in eventSeq (created by checkerOfActionInvoker())
> such that events.js will bind the listener on the DOM node itself on this
> case? I will ask you for review for that. Thanks.

Moving this to Alex as he has more context about the mochitests and their harness.
Flags: needinfo?(yzenevich) → needinfo?(surkov.alexander)

Comment 60

6 months ago
(In reply to Tim Guan-tin Chien [:timdream] (please needinfo) from comment #52)
> Hi Yura,
> 
> Quick question for you. My patch here will contain click/mouse/touch/pointer
> events into the <video> element itself. 

Could you elaborate please, I'm not following what the change is.

> The change breaks ally test on media
> controls [1] because it binds the event listener on the outer document [2],
> not the element itself.

events.js adds an event listener on a DOM document of a DOM node. What is the outer document you refer to?

> Can I change the object in eventSeq (created by checkerOfActionInvoker())
> such that events.js will bind the listener on the DOM node itself on this
> case?

Listening an event right on a DOM node will probably work overall. I didn't catch though how exactly you want to handle this case separately from all cases.
Flags: needinfo?(surkov.alexander)
Comment hidden (mozreview-request)
Attachment #8846509 - Attachment is obsolete: true
Oops, somehow pushing only the 3rd part will mark the first two parts obsolete. MozReview is fun.

(I guess that won't happen if there is also a MozReview-Commit-ID on Part I)

Anyway, I will need sometime to figure out how I can achieve comment 57. It's a bit over my C++ skill (which is no more than copy-pasting I guess). Will get some crash courses in the office ...

Comment 63

6 months ago
mozreview-review
Comment on attachment 8846508 [details]
Bug 1327097 - Part III, Allow a11y test harness to listen event on the DOM element,

https://reviewboard.mozilla.org/r/119570/#review122536

the patch looks good, I'm not quite following though, why those clicks events cannot be captured or don't bubble up to their document? r=me with the question answered
Attachment #8846508 - Flags: review?(surkov.alexander) → review+
(In reply to alexander :surkov from comment #63)
> the patch looks good, I'm not quite following though, why those clicks
> events cannot be captured or don't bubble up to their document? r=me with
> the question answered

See comment 35. We will need to contain these events in the subtree of the video element (simply, the DOM of video control HTML UIs).
You need to log in before you can comment on or make changes to this bug.