NEW 188574
MotionMark fails to display the "run benchmark" button in some situations
https://bugs.webkit.org/show_bug.cgi?id=188574
Summary MotionMark fails to display the "run benchmark" button in some situations
Fabrice Desré
Reported 2018-08-14 13:17:56 PDT
If window.orientation exists, addOrientationListenerIfNecessary() in animometer.js calls _orientationChanged(). Here it ends up looking for an element: document.querySelector(".start-benchmark p") which is not in the DOM so the classList.XXX fails and benchmarkController.updateStartButtonState() is never called.
Attachments
Patch (4.76 KB, patch)
2018-08-15 12:37 PDT, Said Abou-Hallawa
jonlee: review+
Said Abou-Hallawa
Comment 1 2018-08-15 12:11:11 PDT
The release runner page (index.html and motionmark.js) has the following elements and css: .portrait-orientation-check { display: none; } @media screen and (max-device-width: 1025px) and (orientation: portrait) { .portrait-orientation-check { display: block; } } @media screen and (max-device-width: 1025px) and (orientation: portrait) { .landscape-orientation-check { /* This keeps the button color animation in sync with page, while display: none does not. */ visibility: hidden; } } <p class="portrait-orientation-check"><b>Please rotate your device.</b></p> <button class="landscape-orientation-check" onclick="benchmarkController.startBenchmark()">Run Benchmark</button> -- The <p> element will be hidden by default but will be shown if screen width <= 1025px and the device in portrait mode. -- The <button> element will be shown by default but will be hidden if screen width <= 1025px and the device in portrait mode. The release runner page (developer.html and motionmark.js) has the following elements and code: <div class="start-benchmark"> <p class="hidden">Please rotate the device to orientation before starting.</p> <button id="run-benchmark" onclick="benchmarkController.startBenchmark()">Run benchmark</button> </div> <script> _orientationChanged: function(match) { benchmarkController.isInLandscapeOrientation = match.matches; if (match.matches) document.querySelector(".start-benchmark p").classList.add("hidden"); else document.querySelector(".start-benchmark p").classList.remove("hidden"); benchmarkController.updateStartButtonState(); }, updateStartButtonState: function() { document.getElementById("run-benchmark").disabled = !this.isInLandscapeOrientation; }, </script> So the <p> element ".start-benchmark p" and the <button> "#"run-benchmark" are made hidden or visible based on the result of window.matchMedia("(orientation: landscape). The problem is resources/runner/motionmark.js is shared between the debug and the release runners and both of them make calls to benchmarkController.addOrientationListenerIfNecessary() in their benchmarkController.initialize() function. I think the fix is to make both the debug and the release runners rely on the media query to show or hide the <p> and the <button> element.
Said Abou-Hallawa
Comment 2 2018-08-15 12:37:43 PDT
Jon Lee
Comment 3 2018-08-15 13:24:14 PDT
Comment on attachment 347194 [details] Patch It seems ok, but can we do a spot check on Windows, Android, and other browsers?
Ahmad Saleem
Comment 4 2022-09-03 04:14:24 PDT
This r+ patch landed or needed anymore?
Note You need to log in before you can comment on or make changes to this bug.