From 1f96ff62cf786f93c91e6625bf2b819077902251 Mon Sep 17 00:00:00 2001 From: "Andrew Chen (Eng)" Date: Fri, 24 Aug 2018 13:03:34 -0700 Subject: [PATCH] Fix accessibilityRole value lookup Summary: Accessibility roles are enums that are looked up by matching a string accessibility role from JS to the enum's name using .toUpperCase(). .toUpperCase() causes issues in certain languages such as Turkish because the "i"s translate to "?". D9402330 tried to address this by forcing the .toUpperCase to use Locale.US, but now it sometimes translates to "?" Use .equalsIgnoreCase() instead to avoid translations. Reviewed By: mdvacca, mmmulani Differential Revision: D9497494 fbshipit-source-id: 0f8b7f2071b08ccb86655fee7bfd2d009ddde8eb --- .../uimanager/AccessibilityDelegateUtil.java | 23 +++------ .../react/uimanager/BaseViewManager.java | 11 ++-- .../java/com/facebook/react/uimanager/BUCK | 2 + .../react/uimanager/BaseViewManagerTest.java | 50 +++++++++++++++++++ 4 files changed, 61 insertions(+), 25 deletions(-) create mode 100644 ReactAndroid/src/test/java/com/facebook/react/uimanager/BaseViewManagerTest.java diff --git a/ReactAndroid/src/main/java/com/facebook/react/uimanager/AccessibilityDelegateUtil.java b/ReactAndroid/src/main/java/com/facebook/react/uimanager/AccessibilityDelegateUtil.java index c834c85fd2cd69..4d5918ec85099f 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/uimanager/AccessibilityDelegateUtil.java +++ b/ReactAndroid/src/main/java/com/facebook/react/uimanager/AccessibilityDelegateUtil.java @@ -5,16 +5,12 @@ package com.facebook.react.uimanager; -import android.annotation.TargetApi; import android.content.Context; -import android.os.Build; import android.support.v4.view.AccessibilityDelegateCompat; import android.support.v4.view.ViewCompat; import android.support.v4.view.accessibility.AccessibilityNodeInfoCompat; import android.view.View; -import android.view.accessibility.AccessibilityNodeInfo; import com.facebook.react.R; -import com.facebook.react.bridge.ReadableArray; import java.util.Locale; import javax.annotation.Nullable; @@ -58,11 +54,11 @@ public String getValue() { public static AccessibilityRole fromValue(String value) { for (AccessibilityRole role : AccessibilityRole.values()) { - if (role.getValue() != null && role.getValue().equals(value)) { + if (role.name().equalsIgnoreCase(value)) { return role; } } - return AccessibilityRole.NONE; + throw new IllegalArgumentException("Invalid accessibility role value: " + value); } } @@ -82,7 +78,10 @@ public void onInitializeAccessibilityNodeInfo( View host, AccessibilityNodeInfoCompat info) { super.onInitializeAccessibilityNodeInfo(host, info); String accessibilityHint = (String) view.getTag(R.id.accessibility_hint); - AccessibilityRole accessibilityRole = getAccessibilityRole((String) view.getTag(R.id.accessibility_role)); + AccessibilityRole accessibilityRole = (AccessibilityRole) view.getTag(R.id.accessibility_role); + if (accessibilityRole == null) { + accessibilityRole = AccessibilityRole.NONE; + } setRole(info, accessibilityRole, view.getContext()); if (!(accessibilityHint == null)) { String contentDescription=(String)info.getContentDescription(); @@ -127,14 +126,4 @@ public static void setRole(AccessibilityNodeInfoCompat nodeInfo, final Accessibi nodeInfo.setClickable(true); } } - - /** - * Method for setting accessibilityRole on view properties. - */ - public static AccessibilityRole getAccessibilityRole(String role) { - if (role == null) { - return AccessibilityRole.NONE; - } - return AccessibilityRole.valueOf(role.toUpperCase()); - } } diff --git a/ReactAndroid/src/main/java/com/facebook/react/uimanager/BaseViewManager.java b/ReactAndroid/src/main/java/com/facebook/react/uimanager/BaseViewManager.java index fd5b2de97f6355..e05d85dbfa008b 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/uimanager/BaseViewManager.java +++ b/ReactAndroid/src/main/java/com/facebook/react/uimanager/BaseViewManager.java @@ -11,6 +11,7 @@ import android.view.ViewParent; import com.facebook.react.R; import com.facebook.react.bridge.ReadableArray; +import com.facebook.react.uimanager.AccessibilityDelegateUtil.AccessibilityRole; import com.facebook.react.uimanager.annotations.ReactProp; import com.facebook.react.uimanager.util.ReactFindViewUtil; import java.util.Locale; @@ -131,14 +132,8 @@ public void setAccessibilityRole(T view, String accessibilityRole) { if (accessibilityRole == null) { return; } - try { - AccessibilityDelegateUtil.AccessibilityRole.valueOf(accessibilityRole.toUpperCase(Locale.US)); - } catch (NullPointerException e) { - throw new IllegalArgumentException("Invalid Role " + accessibilityRole + " Passed In"); - } catch (IllegalArgumentException e) { - throw new IllegalArgumentException("Invalid Role " + accessibilityRole + " Passed In"); - } - view.setTag(R.id.accessibility_role, accessibilityRole); + + view.setTag(R.id.accessibility_role, AccessibilityRole.fromValue(accessibilityRole)); } @ReactProp(name = PROP_ACCESSIBILITY_STATES) diff --git a/ReactAndroid/src/test/java/com/facebook/react/uimanager/BUCK b/ReactAndroid/src/test/java/com/facebook/react/uimanager/BUCK index c51368c734b266..becfa55ea6b242 100644 --- a/ReactAndroid/src/test/java/com/facebook/react/uimanager/BUCK +++ b/ReactAndroid/src/test/java/com/facebook/react/uimanager/BUCK @@ -5,6 +5,7 @@ rn_robolectric_test( # TODO Disabled temporarily until Yoga linking is fixed t14964130 # srcs = glob(['**/*.java']), srcs = [ + "BaseViewManagerTest.java", "MatrixMathHelperTest.java", "SimpleViewPropertyTest.java", ], @@ -33,6 +34,7 @@ rn_robolectric_test( react_native_target("java/com/facebook/react/uimanager/annotations:annotations"), react_native_target("java/com/facebook/react/views/text:text"), react_native_target("java/com/facebook/react/views/view:view"), + react_native_target("res:uimanager"), react_native_tests_target("java/com/facebook/react/bridge:testhelpers"), ], ) diff --git a/ReactAndroid/src/test/java/com/facebook/react/uimanager/BaseViewManagerTest.java b/ReactAndroid/src/test/java/com/facebook/react/uimanager/BaseViewManagerTest.java new file mode 100644 index 00000000000000..58431477175f28 --- /dev/null +++ b/ReactAndroid/src/test/java/com/facebook/react/uimanager/BaseViewManagerTest.java @@ -0,0 +1,50 @@ +/** + * Copyright (c) 2015-present, Facebook, Inc. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +package com.facebook.react.uimanager; + +import static org.mockito.Mockito.mock; + +import android.content.Context; +import android.support.v4.view.ViewCompat; +import com.facebook.react.uimanager.AccessibilityDelegateUtil.AccessibilityRole; +import com.facebook.react.views.view.ReactViewGroup; +import com.facebook.react.views.view.ReactViewManager; +import com.facebook.react.R; +import java.util.Locale; +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.robolectric.RuntimeEnvironment; +import org.robolectric.RobolectricTestRunner; +import static org.fest.assertions.api.Assertions.assertThat; + +@RunWith(RobolectricTestRunner.class) +public class BaseViewManagerTest { + + private BaseViewManager mViewManager; + private ReactViewGroup mView; + + @Before + public void setUp() { + mViewManager = new ReactViewManager(); + mView = new ReactViewGroup(RuntimeEnvironment.application); + } + + @Test + public void testAccessibilityRoleNone() { + mViewManager.setAccessibilityRole(mView, "none"); + assertThat(mView.getTag(R.id.accessibility_role)).isEqualTo(AccessibilityRole.NONE); + } + + @Test + public void testAccessibilityRoleTurkish() { + Locale.setDefault(Locale.forLanguageTag("tr-TR")); + mViewManager.setAccessibilityRole(mView, "image"); + assertThat(mView.getTag(R.id.accessibility_role)).isEqualTo(AccessibilityRole.IMAGE); + } +}