Skip to content
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

Fix concatenation of accessibility labels in automated tests on iOS #24113

Closed
wants to merge 4 commits into from
Closed

Fix concatenation of accessibility labels in automated tests on iOS #24113

wants to merge 4 commits into from

Conversation

LorandP
Copy link

@LorandP LorandP commented Mar 23, 2019

Summary

When using Appium in order to provide automated tests a unique accessibility label has to be specified for a single element in order to be identified in the Appium inspector tool. However, on iOS, the accessibility label for a single element contained the labels of all the elements on the screen concatenated into one string.

I have provided a fix that would stop concatenating the accessibility labels of other elements from the screen.

Changelog

[IOS][Changed] - Fix concatenation of accessibilityLabels for an element

Test Plan

Before the fix, the accessibility labels were showing like this in the Appium inspector.
https://ibb.co/Nxq4Cdh

I have run the following tests in order to make sure that the current modifications are working and that the old functionality is still working.

  1. I have run the integration test locally from ./scripts/objc-test-ios.sh and the results have passed
  2. I have also run the RNTester module on a simulator and all the components were working as expected
  3. We have run automated tests on our local changes and it has been confirmed that the accessibility labels were no longer concatenated and that every element contained the label that was set in the project.

@LorandP LorandP requested a review from shergin as a code owner March 23, 2019 15:14
@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Mar 23, 2019
@LorandP LorandP closed this Mar 24, 2019
@LorandP LorandP deleted the fix/accessibility-label-on-iOS branch March 24, 2019 10:25
nfriedly added a commit to nfriedly/react-native that referenced this pull request Aug 25, 2020
I came across this because one of my customer's apps was freezing the UI and crashing after spending ~10 seconds when trying to read the accessibilityLabel of a view with a "particularly complex and dynamic view hierarchy". I'm still not entirely sure what the app was doing, but I believe this will sidestep the issue entirely.

I believe this change will also improve both the accessibility and the testability of RN apps, in addition to fixing that crash.

This PR supersedes or fixes facebook#21830, facebook#25963, facebook#24113, facebook#24118, appium/appium#10654, possibly facebook#25220, and probably a few other tickets I haven't identified.

Also worth mentioning that there is very similar code in https://github.com/facebook/react-native/blob/master/React/Fabric/Mounting/ComponentViews/View/RCTViewComponentView.mm#L489-L515 - I haven't dug into that as much, but I suspect it should probably get the same treatment. If anyone wants, I can include it in this PR.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Platform: iOS iOS applications.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants