-
Notifications
You must be signed in to change notification settings - Fork 6.8k
ci: update remote browsers in saucelabs and browserstack #24468
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
Merged
andrewseguin
merged 4 commits into
angular:master
from
devversion:ci/update-remote-browser-tests
Feb 24, 2022
Merged
ci: update remote browsers in saucelabs and browserstack #24468
andrewseguin
merged 4 commits into
angular:master
from
devversion:ci/update-remote-browser-tests
Feb 24, 2022
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Updates the remote browsers in SL and BS. We also updated Chromium and Firefox recently through `dev-infra`.
Updates the CDK table flex-layout sticky to work with Safari v15. The test starts failing because the `position: sticky` does not work as expected for the custom elements without an explicit `display`. Updating the fixture to actually align columns in a row horizontally fixes this and makes sense, given the name of the fixture being about flex-based table.
1caae0b
to
e3b6fc9
Compare
…on disabled elements Currently the CDK harness implementation for `TestBed` tries to mimic user interactions, like `click`. These helpers can then be called by harness authors or consumers. Currently there is some inconistency across harness environments where `testElement.click()` on disabled buttons, inputs etc. causes `click` events to be still emitted in `TestBed` for Chromium-based browsers, while this is not the case for `TestBed` in `Firefox` and `Safari`, neither in e2e-based clicks (like protractor or webdriver). This seems to be inconsistency caused by browsers in the ecosystem. There are bug reports in Firefox where they explicitly kept emitting `click` when explicitly dispatched programmatically (seems reasonable to me). Chromium has a bug report where users complain about events being prevented, even though dispatched programmatically. https://bugzilla.mozilla.org/show_bug.cgi?id=329509. https://bugs.chromium.org/p/chromium/issues/detail?id=1115661 In either way though, we should make this consistent with an actual user interaction (like with e2e environments) and just skip the click event when the underlying element is disabled (we assume the `disabled` property is a sufficient indicator for this). Note that the mousedown, mouseup event sequence continues to be emulated/synthesized.
…earch Some tests seem to use `:focus` instead of `document.activeElement` for test assertions. This seems to work unreliably in Safari as sometimes the `:focus` query returns `null`, unless `document.activeElement` is requested before (which seems to indicate that `document.activeElement` does something under-the-hood to determine the active element, while `querySelector(':focus')` does not have this. The details on why `activeElement` is better in our case are a little hypothetical but doesn't seem worth investigating a lot since in all other places in the repo we haven't used this pattern for checking the active document element.
e1d09b3
to
765b407
Compare
It would be interesting seeing how many tests this breaks inside g3 (looking at the |
andrewseguin
approved these changes
Feb 23, 2022
andrewseguin
pushed a commit
that referenced
this pull request
Feb 24, 2022
* ci: update remote browsers in saucelabs and browserstack Updates the remote browsers in SL and BS. We also updated Chromium and Firefox recently through `dev-infra`. * test: update cdk table sticky test to work with Safari v15 Updates the CDK table flex-layout sticky to work with Safari v15. The test starts failing because the `position: sticky` does not work as expected for the custom elements without an explicit `display`. Updating the fixture to actually align columns in a row horizontally fixes this and makes sense, given the name of the fixture being about flex-based table. * fix(cdk/testing): harness click events inconsistently causing clicks on disabled elements Currently the CDK harness implementation for `TestBed` tries to mimic user interactions, like `click`. These helpers can then be called by harness authors or consumers. Currently there is some inconistency across harness environments where `testElement.click()` on disabled buttons, inputs etc. causes `click` events to be still emitted in `TestBed` for Chromium-based browsers, while this is not the case for `TestBed` in `Firefox` and `Safari`, neither in e2e-based clicks (like protractor or webdriver). This seems to be inconsistency caused by browsers in the ecosystem. There are bug reports in Firefox where they explicitly kept emitting `click` when explicitly dispatched programmatically (seems reasonable to me). Chromium has a bug report where users complain about events being prevented, even though dispatched programmatically. https://bugzilla.mozilla.org/show_bug.cgi?id=329509. https://bugs.chromium.org/p/chromium/issues/detail?id=1115661 In either way though, we should make this consistent with an actual user interaction (like with e2e environments) and just skip the click event when the underlying element is disabled (we assume the `disabled` property is a sufficient indicator for this). Note that the mousedown, mouseup event sequence continues to be emulated/synthesized. * test: use `document.activeElement` instead of pseudo `:focus` query search Some tests seem to use `:focus` instead of `document.activeElement` for test assertions. This seems to work unreliably in Safari as sometimes the `:focus` query returns `null`, unless `document.activeElement` is requested before (which seems to indicate that `document.activeElement` does something under-the-hood to determine the active element, while `querySelector(':focus')` does not have this. The details on why `activeElement` is better in our case are a little hypothetical but doesn't seem worth investigating a lot since in all other places in the repo we haven't used this pattern for checking the active document element. (cherry picked from commit cf79308)
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Labels
action: merge
The PR is ready for merge by the caretaker
target: patch
This PR is targeted for the next patch release
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
See individual commits.
TL;DR: Updates us to the latest available Safari for iOS and macOS. Adjusts tests as needed, and removed some test filter that disabled certain tests in
Firefox
that actually were hiding an underlying issue incdk/testing
.