Fix accessibility label concatenation on iOS #24118
Closed
Conversation
This file contains 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
facebook-github-bot
added
the
CLA Signed
|
@rigdern What do you thing? |
|
@shergin Hy is there a way in which I can see the entire test that is being run by circleci for the test_detox_end_to_end? I have detox setup locally and I would like to reproduce the error in order to fix that failed test. |
|
@LorandP TBH, |
|
I am open to discussing this thing, but this implementation is incorrect because using |
|
I agree I will try to find a better solution for this and if you have any suggestions just let me know. Thanks for the feedback. |
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.
|
Any progress on this one? I am still experiencing the same issue :/ |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
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.
Also, other issues opened on this:
#21830
appium/appium#10654
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.