Add a way to reset the eventTrigger in ControlsTest in LayoutTests/media/controls/controls-test-helpers.js We can introduce a method in ControlsTest called listenFor(eventTrigger), that sets the ControlsTest's eventTrigger to the new value and set the media element to be a listener to that event. This makes it easier to write controls tests that need to listen to a new event after changing some state in the media element.
Created attachment 272065 [details] Patch
Comment on attachment 272065 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=272065&action=review > LayoutTests/media/controls/controls-test-helpers.js:31 > + this.eventTrigger = eventTrigger; > + if (!this.media) > + return null; > + > + this.media.addEventListener(this.eventTrigger, this, false); > + return this; Don’t we need to removeEventListener on the old value of this.eventTrigger?
(In reply to comment #2) > Comment on attachment 272065 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=272065&action=review > > > LayoutTests/media/controls/controls-test-helpers.js:31 > > + this.eventTrigger = eventTrigger; > > + if (!this.media) > > + return null; > > + > > + this.media.addEventListener(this.eventTrigger, this, false); > > + return this; > > Don’t we need to removeEventListener on the old value of this.eventTrigger? That's a good question. I think you are right. Thinking about this a little more, instead of introducing a listenFor() function, maybe I should just implement the setter of eventTrigger to do the right thing. I'll talk more with Dean about this.
Created attachment 275153 [details] Patch
Uploaded a patch that renames the method and also call removeEventListener on the old event trigger.
Comment on attachment 275153 [details] Patch Very nice! One of the disadvantages of this chaining().style().of().coding() that I used here is that it makes it harder to use getters and setters. In some ways this method could have been tester.eventTrigger = foo, but I think what you did is good.
Committed: http://trac.webkit.org/changeset/198949