154616 2016-02-23 16:44:26 -0800 Add a way to reset the eventTrigger in ControlsTest in LayoutTests/media/controls/controls-test-helpers.js 2016-04-01 11:15:30 -0700 1 1 1 Unclassified WebKit Tools / Tests WebKit Nightly Build Unspecified Unspecified RESOLVED FIXED P2 Normal --- 1 adachan adachan dino eric.carlson lforschler oldest_to_newest 1167594 0 adachan 2016-02-23 16:44:26 -0800 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. 1167597 1 272065 adachan 2016-02-23 16:48:36 -0800 Created attachment 272065 Patch 1167676 2 272065 darin 2016-02-23 21:54:43 -0800 Comment on attachment 272065 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? 1167820 3 adachan 2016-02-24 10:17:17 -0800 (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. 1178870 4 275153 adachan 2016-03-29 16:57:42 -0700 Created attachment 275153 Patch 1178872 5 adachan 2016-03-29 17:04:35 -0700 Uploaded a patch that renames the method and also call removeEventListener on the old event trigger. 1178884 6 275153 dino 2016-03-29 17:34:27 -0700 Comment on attachment 275153 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. 1179894 7 adachan 2016-04-01 11:15:30 -0700 Committed: http://trac.webkit.org/changeset/198949 272065 2016-02-23 16:48:36 -0800 2016-03-29 16:57:39 -0700 Patch bug-154616-20160223164821.patch text/plain 1427 adachan U3VidmVyc2lvbiBSZXZpc2lvbjogMTk3MDA1CmRpZmYgLS1naXQgYS9MYXlvdXRUZXN0cy9DaGFu Z2VMb2cgYi9MYXlvdXRUZXN0cy9DaGFuZ2VMb2cKaW5kZXggYWIxOWNhYzkzNjUxOTYwZWRkMGQ1 YjE2MGVlYjE5N2E1MTZmY2ZhMy4uY2YyNzgyYWY4OWNkMzUyMDkwNGFlNWQ3OGU4M2VmYjIzZjFi ZDYxZCAxMDA2NDQKLS0tIGEvTGF5b3V0VGVzdHMvQ2hhbmdlTG9nCisrKyBiL0xheW91dFRlc3Rz L0NoYW5nZUxvZwpAQCAtMSwzICsxLDEzIEBACisyMDE2LTAyLTIzICBBZGEgQ2hhbiAgPGFkYWNo YW5AYXBwbGUuY29tPgorCisgICAgICAgIEFkZCBhIHdheSB0byByZXNldCB0aGUgZXZlbnRUcmln Z2VyIGluIENvbnRyb2xzVGVzdCBpbiBMYXlvdXRUZXN0cy9tZWRpYS9jb250cm9scy9jb250cm9s cy10ZXN0LWhlbHBlcnMuanMKKyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19i dWcuY2dpP2lkPTE1NDYxNgorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgor CisgICAgICAgICogbWVkaWEvY29udHJvbHMvY29udHJvbHMtdGVzdC1oZWxwZXJzLmpzOgorICAg ICAgICAoQ29udHJvbHNUZXN0LnByb3RvdHlwZS5saXN0ZW5Gb3IpOgorCiAyMDE2LTAyLTIyICBS eW9zdWtlIE5pd2EgIDxybml3YUB3ZWJraXQub3JnPgogCiAgICAgICAgIENhbGxpbmcgaW1wb3J0 Tm9kZSBvbiBzaGFkb3cgcm9vdCBjYXVzZXMgYSBjcmFzaApkaWZmIC0tZ2l0IGEvTGF5b3V0VGVz dHMvbWVkaWEvY29udHJvbHMvY29udHJvbHMtdGVzdC1oZWxwZXJzLmpzIGIvTGF5b3V0VGVzdHMv bWVkaWEvY29udHJvbHMvY29udHJvbHMtdGVzdC1oZWxwZXJzLmpzCmluZGV4IDU5NDJkOTFkMTI5 YjMyOWM0MWNhMDc1MWIzYmQzYjNhMjcyMjRkYjUuLmI2Mzg4ODk1YTIzYWRjMzhkN2ViZmIyOTMx NTg0NjE0OGI3OTYyYzQgMTAwNjQ0Ci0tLSBhL0xheW91dFRlc3RzL21lZGlhL2NvbnRyb2xzL2Nv bnRyb2xzLXRlc3QtaGVscGVycy5qcworKysgYi9MYXlvdXRUZXN0cy9tZWRpYS9jb250cm9scy9j b250cm9scy10ZXN0LWhlbHBlcnMuanMKQEAgLTIxLDYgKzIxLDE2IEBAIENvbnRyb2xzVGVzdCA9 IGNsYXNzIENvbnRyb2xzVGVzdCB7CiAgICAgICAgIHJldHVybiB0aGlzOwogICAgIH0KIAorICAg IGxpc3RlbkZvcihldmVudFRyaWdnZXIpCisgICAgeworICAgICAgICB0aGlzLmV2ZW50VHJpZ2dl ciA9IGV2ZW50VHJpZ2dlcjsKKyAgICAgICAgaWYgKCF0aGlzLm1lZGlhKQorICAgICAgICAgICAg cmV0dXJuIG51bGw7CisKKyAgICAgICAgdGhpcy5tZWRpYS5hZGRFdmVudExpc3RlbmVyKHRoaXMu ZXZlbnRUcmlnZ2VyLCB0aGlzLCBmYWxzZSk7CisgICAgICAgIHJldHVybiB0aGlzOworICAgIH0K KwogICAgIGdldCBjdXJyZW50U3RhdGUoKQogICAgIHsKICAgICAgICAgaWYgKCF0aGlzLm1lZGlh KQo= 275153 2016-03-29 16:57:42 -0700 2016-03-29 17:34:27 -0700 Patch bug-154616-20160329165745.patch text/plain 1576 adachan U3VidmVyc2lvbiBSZXZpc2lvbjogMTk4Nzk3CmRpZmYgLS1naXQgYS9MYXlvdXRUZXN0cy9DaGFu Z2VMb2cgYi9MYXlvdXRUZXN0cy9DaGFuZ2VMb2cKaW5kZXggYTFkYzcxNDVhNDE3N2EyZmZiOGY0 ODg0YmQ3MmE4NzczNWQyYWZiOS4uMTY3ZWI0N2E0ZTA3NDZhNTdlZTE1YjRlZTYxN2JmZGE1MTUw NWZmNCAxMDA2NDQKLS0tIGEvTGF5b3V0VGVzdHMvQ2hhbmdlTG9nCisrKyBiL0xheW91dFRlc3Rz L0NoYW5nZUxvZwpAQCAtMSwzICsxLDEzIEBACisyMDE2LTAzLTI5ICBBZGEgQ2hhbiAgPGFkYWNo YW5AYXBwbGUuY29tPgorCisgICAgICAgIEFkZCBhIHdheSB0byByZXNldCB0aGUgZXZlbnRUcmln Z2VyIGluIENvbnRyb2xzVGVzdCBpbiBMYXlvdXRUZXN0cy9tZWRpYS9jb250cm9scy9jb250cm9s cy10ZXN0LWhlbHBlcnMuanMKKyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19i dWcuY2dpP2lkPTE1NDYxNgorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgor CisgICAgICAgICogbWVkaWEvY29udHJvbHMvY29udHJvbHMtdGVzdC1oZWxwZXJzLmpzOgorICAg ICAgICAoQ29udHJvbHNUZXN0LnByb3RvdHlwZS5yZXNldEV2ZW50VHJpZ2dlcik6CisKIDIwMTYt MDMtMjkgIFNhYW0gYmFyYXRpICA8c2JhcmF0aUBhcHBsZS5jb20+CiAKICAgICAgICAgIkNhbiBu b3QiID0+ICJjYW5ub3QiIGluIFN0cmluZy5wcm90b3R5cGUgZXJyb3IgbWVzc2FnZXMKZGlmZiAt LWdpdCBhL0xheW91dFRlc3RzL21lZGlhL2NvbnRyb2xzL2NvbnRyb2xzLXRlc3QtaGVscGVycy5q cyBiL0xheW91dFRlc3RzL21lZGlhL2NvbnRyb2xzL2NvbnRyb2xzLXRlc3QtaGVscGVycy5qcwpp bmRleCA1OTQyZDkxZDEyOWIzMjljNDFjYTA3NTFiM2JkM2IzYTI3MjI0ZGI1Li5jZjk5NjMxZDM5 MWZiNGJhYjU4MGM1NDhhZTE5Mzg0Zjg5MDFmYzZjIDEwMDY0NAotLS0gYS9MYXlvdXRUZXN0cy9t ZWRpYS9jb250cm9scy9jb250cm9scy10ZXN0LWhlbHBlcnMuanMKKysrIGIvTGF5b3V0VGVzdHMv bWVkaWEvY29udHJvbHMvY29udHJvbHMtdGVzdC1oZWxwZXJzLmpzCkBAIC0yMSw2ICsyMSwxOSBA QCBDb250cm9sc1Rlc3QgPSBjbGFzcyBDb250cm9sc1Rlc3QgewogICAgICAgICByZXR1cm4gdGhp czsKICAgICB9CiAKKyAgICByZXNldEV2ZW50VHJpZ2dlcihldmVudFRyaWdnZXIpCisgICAgewor ICAgICAgICBpZiAodGhpcy5ldmVudFRyaWdnZXIgJiYgdGhpcy5tZWRpYSkKKyAgICAgICAgICAg IHRoaXMubWVkaWEucmVtb3ZlRXZlbnRMaXN0ZW5lcih0aGlzLmV2ZW50VHJpZ2dlciwgdGhpcywg ZmFsc2UpOworCisgICAgICAgIHRoaXMuZXZlbnRUcmlnZ2VyID0gZXZlbnRUcmlnZ2VyOworICAg ICAgICBpZiAoIXRoaXMubWVkaWEpCisgICAgICAgICAgICByZXR1cm4gbnVsbDsKKworICAgICAg ICB0aGlzLm1lZGlhLmFkZEV2ZW50TGlzdGVuZXIodGhpcy5ldmVudFRyaWdnZXIsIHRoaXMsIGZh bHNlKTsKKyAgICAgICAgcmV0dXJuIHRoaXM7CisgICAgfQorCiAgICAgZ2V0IGN1cnJlbnRTdGF0 ZSgpCiAgICAgewogICAgICAgICBpZiAoIXRoaXMubWVkaWEpCg==