Join GitHub today
GitHub is home to over 50 million developers working together to host and review code, manage projects, and build software together.
Sign upAdd a reftest that entering fullscreen mode takes over the window #22809
Conversation
highfive
commented
Feb 1, 2019
|
Heads up! This PR modifies the following files:
|
|
Opened new PR for upstreamable changes. Completed upstream sync of web-platform-test changes at web-platform-tests/wpt#15218. |
|
This is a dry run for reftests for WebVR/XR which look a lot like putting a canvas into fullscreen mode. |
tests/wpt/web-platform-tests/fullscreen/reftests/fullscreen-baseline.html
Outdated
Show resolved
Hide resolved
jugglinmike
commented
Feb 2, 2019
|
We'd like to get testdriver.js working in reftests for cases like these, but we're not there yet. @jdm the tests in this patch have already been submitted to WPT and labeled as "already reviewed downstream". Is that a bug? |
|
No, it's how our system works. It gets merged upstream after it merges in this repository. |
jugglinmike
commented
Feb 2, 2019
|
Ah, got it. Maybe we should change @wpt-pr-bot's message a bit and/or delay approval. |
|
I'll point out that the Chromium upstreaming system behaves the exact same way. The presence of the 'do not merge' label acts as an indicator of the status. |
|
Should I add a README about what's needed to pass this test? |
|
No. We shouldn't add upstream tests that other browsers can't pass. |
|
Okay, I'll move the reftest into the wpt/mozilla directory then. At some point the other browsers might like some reftests for fullscreen, but we can deal with that when there's an agreed way to do the testing. |
|
No upstreamable changes; closed existing PR. Completed upstream sync of web-platform-test changes at web-platform-tests/wpt#15218. |
|
Okay, test moved to the |
asajeffrey
force-pushed the
asajeffrey:wpt-add-fullscreen-baseline-reftest
branch
from
bd73cab
to
f1d0f73
|
This looks ready to merge as soon as the conflict is gone. |
asajeffrey
force-pushed the
asajeffrey:wpt-add-fullscreen-baseline-reftest
branch
from
f1d0f73
to
d286209
|
@bors-servo r+ |
|
|
|
Argh I meant r=jdm, oops. |
…, r=asajeffrey Add a reftest that entering fullscreen mode takes over the window <!-- Please describe your changes on the following line: --> Add a baseline reftest for fullscreen that ensures that going fullscreen actually uses the whole window. This uses a pref that requests that fullscreen mode takes over the window rather than the whole screen. The pref doesn't do anything right now, but is a bit of future-proofing for when we do have proper fullscreen. --- <!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `___` with appropriate data: --> - [X] `./mach build -d` does not report any errors - [X] `./mach test-tidy` does not report any errors - [X] These changes fix #22804. - [X] There are tests for these changes <!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.--> <!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. --> <!-- Reviewable:start --> --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/22809) <!-- Reviewable:end -->
|
|
|
@bors-servo retry |
|
|
|
|
Some downstream projects create a pull request before code review has been completed [1]. Update the generated comment to avoid suggesting otherwise, and extend with the name of the downstream project where review is under way. [1] servo/servo#22809 (comment)
Some downstream projects create a pull request before code review has been completed [1]. Update the generated comment to avoid suggesting otherwise, and extend with the name of the downstream project where review is under way. [1] servo/servo#22809 (comment)
Some downstream projects create a pull request before code review has been completed [1]. Update the generated comment to avoid suggesting otherwise, and extend with the name of the downstream project where review is under way. [1] servo/servo#22809 (comment)
asajeffrey commentedFeb 1, 2019
•
edited by SimonSapin
Add a baseline reftest for fullscreen that ensures that going fullscreen actually uses the whole window. This uses a pref that requests that fullscreen mode takes over the window rather than the whole screen. The pref doesn't do anything right now, but is a bit of future-proofing for when we do have proper fullscreen.
./mach build -ddoes not report any errors./mach test-tidydoes not report any errorsThis change is