1350927 - consider converting media controls' direction mochitests to regular reftest

Status: NEW

(Toolkit :: Video/Audio Controls, enhancement)

Description

[Ray Lin[:ralin]]

Instead of doing something similar to reftest in direction mochitests with canvas.compare(), we should consider converting them to regular reftests if possible.


[0] https://dxr.mozilla.org/mozilla-central/rev/e03e0c60462c775c7558a1dc9d5cf2076c3cd1f9/toolkit/content/tests/widgets/videocontrols_direction_test.js#52-84

Comment 1

[Daniel Holbert [:dholbert]]

So the mochitests in question are...
https://dxr.mozilla.org/mozilla-central/source/toolkit/content/tests/widgets/test_videocontrols_audio_direction.html
https://dxr.mozilla.org/mozilla-central/source/toolkit/content/tests/widgets/test_videocontrols_video_direction.html
...and they load reftests like this one:
https://dxr.mozilla.org/mozilla-central/source/toolkit/content/tests/widgets/videocontrols_direction-1a.html
...and they use the js script in comment 0 to "drive" them (to load the specified testcases & do the comparisons).

Looks like these tests were originally added as mochitests by ehsan in bug 489631, ages ago. Adding dependency.

(It's not immediately clear to me why we're using mochitests here instead of reftests. ralin, maybe you can tell?  or perhaps ehsan remembers?)

Also: ralin, if your primary goal here is to remove the "mask" elements (videomask.css) and use fuzzy-matching instead (as discussed in bug 1347673 comment 14 & 17), you can do that without needing to convert to reftests. We'd just need to rewrite the mochitest to use WindowSnapshot.js to do the snapshots & canvas comparisons (with either compareSnapshots or assertSnapshots), and make use of the "fuzz" parameter there.  Here's that JS library for reference:
https://dxr.mozilla.org/mozilla-central/source/testing/mochitest/tests/SimpleTest/WindowSnapshot.js#22

That's perhaps a less invasive change than converting to reftests.  (Though on the other hand, if there's no reason these can't just be reftests now, then maybe we should just make them reftests.)

Comment 2

[Daniel Holbert [:dholbert]]

(In reply to Daniel Holbert [:dholbert] from comment #1)
> (It's not immediately clear to me why we're using mochitests here instead of
> reftests. ralin, maybe you can tell?  or perhaps ehsan remembers?)

Separate thing I was wondering: why did we originally need a mask to cover up part of the center of the video in these tests?

That's explained here in this helper file:
>  * Create a mask for the video direction tests which covers up the throbber.
>  */
https://dxr.mozilla.org/mozilla-central/rev/e03e0c60462c775c7558a1dc9d5cf2076c3cd1f9/toolkit/content/tests/widgets/videomask.css#12

So I guess (and I think I recall) we used to show a throbber over videos while the data was loading, and so an arbitrarily-timed snapshot would get a random sample of the throbber graphic, which couldn't be expected to match another snapshot of the throbber.

I don't think we show that throbber anymore, though, so maybe this mask can go away? (except that *maybe* there's unrelated fuzziness on the giant play button, the same fuzziness that ralin encountered in bug 1347673 comment 11)