[mediacontrols] Improve media controls testing helpers
<rdar://problem/22530876>
Created attachment 260396 [details] Patch
Comment on attachment 260396 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=260396&action=review Very nice! > LayoutTests/media/controls/basic.html:9 > + var tester = new ControlsTest(); > + tester.whenReady(runTests); > + tester.start(); Can you make this even more modern-looking with something like: var tester = new ControlsTest() .whenReady(runTests) .start(); :-) > LayoutTests/media/controls/controls-test-helpers.js:11 > + this.eventType = eventType || "canplaythrough"; Nit: maybe 'eventTrigger' instead of 'eventType'? > LayoutTests/media/controls/controls-test-helpers.js:66 > + var textMsg = document.createTextNode(msg); > + this.console.appendChild(textMsg); > + var br = document.createElement("br"); > + this.console.appendChild(br); Nit: don't really need the local variables. > LayoutTests/media/controls/controls-test-helpers.js:73 > + this.logMessage(""); > + this.logMessage(msg); > + this.logMessage(""); Nit: "this.logMessage('<br>' + msg + '<br>') would be more concise. > LayoutTests/media/controls/showControlsButton-expected.txt:12 > +EVENT: canplaythrough > +PASS: We are using the apple idiom > + > +Get the button that shows controls while in Voice Over > + > +PASS: Name is 'Show Controls' > +PASS: Is hidden It would be nice to have a label for the first test section as well. Maybe have the constructor take a label and have start() call startNewSection()?.
Comment on attachment 260396 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=260396&action=review >> LayoutTests/media/controls/basic.html:9 >> + tester.start(); > > Can you make this even more modern-looking with something like: > > var tester = new ControlsTest() > .whenReady(runTests) > .start(); > > :-) Done! >> LayoutTests/media/controls/controls-test-helpers.js:11 >> + this.eventType = eventType || "canplaythrough"; > > Nit: maybe 'eventTrigger' instead of 'eventType'? Changed! >> LayoutTests/media/controls/controls-test-helpers.js:66 >> + this.console.appendChild(br); > > Nit: don't really need the local variables. Removed! >> LayoutTests/media/controls/controls-test-helpers.js:73 >> + this.logMessage(""); > > Nit: "this.logMessage('<br>' + msg + '<br>') would be more concise. That doesn't quite work because we append a text node (rather than innerHTML). I cleaned it up a bit though. >> LayoutTests/media/controls/showControlsButton-expected.txt:12 >> +PASS: Is hidden > > It would be nice to have a label for the first test section as well. Maybe have the constructor take a label and have start() call startNewSection()?. Done. The start() method now takes an optional label.
Committed r189254: <http://trac.webkit.org/changeset/189254>