Bug 1284372 - URL bar: Force LTR for the URL. r=grisha, a=sylvestre
authorSebastian Kaspari <s.kaspari@gmail.com>
Wed, 06 Jul 2016 13:57:19 +0200
changeset 341990 c067813334d36db5117665c703f1732739987e76
parent 341989 3eb6b78ba0cffd9befbc5277dd7e36c2cd04008d
child 341991 aa52fb6664b5eead467e1b9d27ac8b52a05e9b55
push id1183
push userraliiev@mozilla.com
push dateMon, 05 Sep 2016 20:01:49 +0000
treeherdermozilla-release@3148731bed45 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersgrisha, sylvestre
bugs1284372
milestone49.0a2
Bug 1284372 - URL bar: Force LTR for the URL. r=grisha, a=sylvestre MozReview-Commit-ID: 3WnPTRmk5VO
mobile/android/base/java/org/mozilla/gecko/toolbar/ToolbarDisplayLayout.java
mobile/android/base/java/org/mozilla/gecko/util/StringUtils.java
mobile/android/tests/background/junit4/src/org/mozilla/gecko/util/TestStringUtils.java
--- a/mobile/android/base/java/org/mozilla/gecko/toolbar/ToolbarDisplayLayout.java
+++ b/mobile/android/base/java/org/mozilla/gecko/toolbar/ToolbarDisplayLayout.java
@@ -273,16 +273,21 @@ public class ToolbarDisplayLayout extend
         String strippedURL = stripAboutReaderURL(url);
 
         final boolean isHttpOrHttps = StringUtils.isHttpOrHttps(strippedURL);
 
         if (mPrefs.shouldTrimUrls()) {
             strippedURL = StringUtils.stripCommonSubdomains(StringUtils.stripScheme(strippedURL));
         }
 
+        // The URL bar does not support RTL currently (See bug 928688 and meta bug 702845).
+        // Displaying a URL using RTL (or mixed) characters can lead to an undesired reordering
+        // of elements of the URL. That's why we are forcing the URL to use LTR (bug 1284372).
+        strippedURL = StringUtils.forceLTR(strippedURL);
+
         // This value is not visible to screen readers but we rely on it when running UI tests. Screen
         // readers will instead focus BrowserToolbar and read the "base domain" from there. UI tests
         // will read the content description to obtain the full URL for performing assertions.
         setContentDescription(strippedURL);
 
         final SiteIdentity siteIdentity = tab.getSiteIdentity();
         if (siteIdentity.hasOwner() && SwitchBoard.isInExperiment(mActivity, Experiments.URLBAR_SHOW_EV_CERT_OWNER)) {
             // Show Owner of EV certificate as title
--- a/mobile/android/base/java/org/mozilla/gecko/util/StringUtils.java
+++ b/mobile/android/base/java/org/mozilla/gecko/util/StringUtils.java
@@ -270,9 +270,37 @@ public class StringUtils {
         return Collections.unmodifiableSet(names);
     }
 
     public static String safeSubstring(@NonNull final String str, final int start, final int end) {
         return str.substring(
                 Math.max(0, start),
                 Math.min(end, str.length()));
     }
+
+    /**
+     * Check if this might be a RTL (right-to-left) text by looking at the first character.
+     */
+    public static boolean isRTL(String text) {
+        if (TextUtils.isEmpty(text)) {
+            return false;
+        }
+
+        final char character = text.charAt(0);
+        final byte directionality = Character.getDirectionality(character);
+
+        return directionality == Character.DIRECTIONALITY_RIGHT_TO_LEFT
+                || directionality == Character.DIRECTIONALITY_RIGHT_TO_LEFT_ARABIC
+                || directionality == Character.DIRECTIONALITY_RIGHT_TO_LEFT_EMBEDDING
+                || directionality == Character.DIRECTIONALITY_RIGHT_TO_LEFT_OVERRIDE;
+    }
+
+    /**
+     * Force LTR (left-to-right) by prepending the text with the "left-to-right mark" (U+200E) if needed.
+     */
+    public static String forceLTR(String text) {
+        if (!isRTL(text)) {
+            return text;
+        }
+
+        return "\u200E" + text;
+    }
 }
--- a/mobile/android/tests/background/junit4/src/org/mozilla/gecko/util/TestStringUtils.java
+++ b/mobile/android/tests/background/junit4/src/org/mozilla/gecko/util/TestStringUtils.java
@@ -7,36 +7,38 @@ package org.mozilla.gecko.util;
 
 import junit.framework.Assert;
 
 import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.mozilla.gecko.background.testhelpers.TestRunner;
 
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
 
 @RunWith(TestRunner.class)
 public class TestStringUtils {
     @Test
     public void testIsHttpOrHttps() {
         // No value
         Assert.assertFalse(StringUtils.isHttpOrHttps(null));
         Assert.assertFalse(StringUtils.isHttpOrHttps(""));
 
         // Garbage
         Assert.assertFalse(StringUtils.isHttpOrHttps("lksdjflasuf"));
 
         // URLs with http/https
-        Assert.assertTrue(StringUtils.isHttpOrHttps("https://www.google.com"));
-        Assert.assertTrue(StringUtils.isHttpOrHttps("http://www.facebook.com"));
-        Assert.assertTrue(StringUtils.isHttpOrHttps("https://mozilla.org/en-US/firefox/products/"));
+        assertTrue(StringUtils.isHttpOrHttps("https://www.google.com"));
+        assertTrue(StringUtils.isHttpOrHttps("http://www.facebook.com"));
+        assertTrue(StringUtils.isHttpOrHttps("https://mozilla.org/en-US/firefox/products/"));
 
         // IP addresses
-        Assert.assertTrue(StringUtils.isHttpOrHttps("https://192.168.0.1"));
-        Assert.assertTrue(StringUtils.isHttpOrHttps("http://63.245.215.20/en-US/firefox/products"));
+        assertTrue(StringUtils.isHttpOrHttps("https://192.168.0.1"));
+        assertTrue(StringUtils.isHttpOrHttps("http://63.245.215.20/en-US/firefox/products"));
 
         // Other protocols
         Assert.assertFalse(StringUtils.isHttpOrHttps("ftp://people.mozilla.org"));
         Assert.assertFalse(StringUtils.isHttpOrHttps("javascript:window.google.com"));
         Assert.assertFalse(StringUtils.isHttpOrHttps("tel://1234567890"));
 
         // No scheme
         Assert.assertFalse(StringUtils.isHttpOrHttps("google.com"));
@@ -48,9 +50,40 @@ public class TestStringUtils {
         assertEquals(StringUtils.stripRef(null), null);
         assertEquals(StringUtils.stripRef(""), "");
 
         assertEquals(StringUtils.stripRef("??AAABBBCCC"), "??AAABBBCCC");
         assertEquals(StringUtils.stripRef("https://mozilla.org"), "https://mozilla.org");
         assertEquals(StringUtils.stripRef("https://mozilla.org#BBBB"), "https://mozilla.org");
         assertEquals(StringUtils.stripRef("https://mozilla.org/#BBBB"), "https://mozilla.org/");
     }
+
+    @Test
+    public void testIsRTL() {
+        assertFalse(StringUtils.isRTL("mozilla.org"));
+        assertFalse(StringUtils.isRTL("something.عربي"));
+
+        assertTrue(StringUtils.isRTL("عربي"));
+        assertTrue(StringUtils.isRTL("عربي.org"));
+
+        // Text with LTR mark
+        assertFalse(StringUtils.isRTL("\u200EHello"));
+        assertFalse(StringUtils.isRTL("\u200Eعربي"));
+    }
+
+    @Test
+    public void testForceLTR() {
+        assertFalse(StringUtils.isRTL(StringUtils.forceLTR("عربي")));
+        assertFalse(StringUtils.isRTL(StringUtils.forceLTR("عربي.org")));
+
+        // Strings that are already LTR are not modified
+        final String someLtrString = "HelloWorld";
+        assertEquals(someLtrString, StringUtils.forceLTR(someLtrString));
+
+        // We add the LTR mark only once
+        final String someRtlString = "عربي";
+        assertEquals(4, someRtlString.length());
+        final String forcedLtrString = StringUtils.forceLTR(someRtlString);
+        assertEquals(5, forcedLtrString.length());
+        final String forcedAgainLtrString = StringUtils.forceLTR(forcedLtrString);
+        assertEquals(5, forcedAgainLtrString.length());
+    }
 }