-
Notifications
You must be signed in to change notification settings - Fork 782
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
>= 2.9.3 breaks our tests #1437
Comments
Interesting; I took a look and likewise didn't see anything that looks like it would cause those failures. I might see if I can reproduce them on my iPhone when I get some time. |
I tried to reproduce this today. Tested on desktop Safari and mobile Safari but with more recent versions and the tests all passed. Unfortunately, I don't have access to any iOS 11 devices so I'm not sure there's anything more I could do. |
Thanks for trying! It only seems to break on iOS Safari, unfortunately :/ /~https://github.com/twbs/bootstrap/runs/687823607#step:17:62 |
I tried on iOS Safari v13, so it must be that specific version of iOS Safari unfortunately. If anyone can pinpoint what's failing, I'd be happy to fix it / accept a PR. |
Do you have BrowserStack tests or something similar for QUnit itself? If so, you could temporarily add iOS Safari 11. Maybe it's something on our part, but it broke after a QUnit minor release. We are still using QUnit 2.9.2 due to that. |
Yep, we have BrowserStack access via the JS Foundation. Looking into this now. I'll see if the tests pass on iOS/Safari 11. |
Thanks for testing. This seems to hint that it's not an issue with QUnit itself, but I can consistently reproduce the failure after 2.9.2: /~https://github.com/twbs/bootstrap/runs/807411252#step:7:66 |
@XhmikosR OK, so I've checked out /~https://github.com/twbs/bootstrap locally and fetched the same commit as that GitHub CI run (twbs/bootstrap@6b67c96), I then ran the following in a container:
... and then start a static http server on port 4000 and used http://localhost:4000/js/tests/ to run the tests in a few browsers. The BrowserStack tunnel wasn't working with iOS 11 for some reason, but I was able to reproduce the issue on iOS 10 as well: The test in question has its source code at /~https://github.com/twbs/bootstrap/blob/6b67c96637b679f6518db5eebef1ca914040405f/js/tests/unit/modal.js#L341-L350. QUnit.test('should restore focus to toggling element when modal is hidden after having been opened via data-api', function (assert) {
assert.expect(1)
var done = assert.async()
var $toggleBtn = $('<button data-toggle="modal" data-target="#modal-test"/>').appendTo('#qunit-fixture')
$('<div id="modal-test"><div class="contents"><div id="close" data-dismiss="modal"/></div></div>')
.on('hidden.bs.modal', function () {
setTimeout(function () {
assert.ok($(document.activeElement).is($toggleBtn), 'toggling element is once again focused') Looking through the QUnit 2.9.2...2.9.3 diff, the following stood out to me: + [id^=qunit] button {
+ font-size: initial;
+ border: initial;
+ background-color: buttonface;
+ } In particular because the Bootstrap tests involves a I removed these lines from I'll get this fixed and backported :) |
Thank you for digging into this! |
We might not even need the hacks we have there (/~https://github.com/twbs/bootstrap/blob/v4-dev-xmr-deps-qunit/js/tests/index.html#L86) since the early days, I'll see if we need them after getting this sorted and I update to the latest QUnit release :) It's been a long time someone touched those. |
There are currently two places where we use buttons: * The toolbar, with the filter "Go" button and the "Apply" and "Reset" buttons for the module dropdown. * The test results, with the temporal "Abort" button. Fixes #1437.
There are currently two places where we use buttons: * The toolbar, with the filter "Go" button and the "Apply" and "Reset" buttons for the module dropdown. * The test results, with the temporal "Abort" button. Fixes #1437.
Tell us about your runtime:
When v2.9.3 was first released, I tried to update it in our Bootstrap v4-dev branch. We were getting 2 BrowserStack failures, namely iOS 11. I reverted back to 2.9.2 and forgot to report the issue 🙂 Today, I tried updating qunit to v2.10.0 and the same errors happened so I thought I'd report them.
Failed tests from our test suite:
What are you trying to do?
What did you expect to happen?
Tests to pass
What actually happened?
The aforementioned errors.
I tried pinpointing any related changes from compare/2.9.2...2.9.3 without success. Going back to v2.9.2 works fine again.
Unfortunately, running these tests locally with Chrome or Firefox works. The errors only happen on iOS Safari.
Let me know if there's something else you need!
/CC @Johann-S
The text was updated successfully, but these errors were encountered: