WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
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+
Details
Formatted Diff
Diff
View All
Add attachment
proposed patch, testcase, etc.
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
Created
attachment 347194
[details]
Patch
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.
Top of Page
Format For Printing
XML
Clone This Bug