| Summary: | MotionMark fails to display the "run benchmark" button in some situations | ||||||
|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Fabrice Desré <fabrice> | ||||
| Component: | Tools / Tests | Assignee: | Said Abou-Hallawa <sabouhallawa> | ||||
| Status: | NEW --- | ||||||
| Severity: | Normal | CC: | ahmad.saleem792, ap, bfulgham, cdumez, dewei_zhu, ews-watchlist, jonlee, lforschler, mmaxfield, rniwa, sabouhallawa, simon.fraser | ||||
| Priority: | P2 | ||||||
| Version: | WebKit Nightly Build | ||||||
| Hardware: | Unspecified | ||||||
| OS: | Unspecified | ||||||
| Attachments: |
|
||||||
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.
Created attachment 347194 [details]
Patch
Comment on attachment 347194 [details]
Patch
It seems ok, but can we do a spot check on Windows, Android, and other browsers?
This r+ patch landed or needed anymore? |
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.