New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 774926 link

Starred by 1 user

Issue metadata

Status: Verified
Owner:
Closed: Oct 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 2
Type: Bug
Team-Security-UX



Sign in to add a comment

Modal permission prompts don't work properly in RTL

Project Member Reported by timloh@chromium.org, Oct 16 2017

Issue description

Attached is camera permissions in Hebrew, infobar vs modal. With modals, the icon is on the wrong side, and the text looks broken (on permissions where the message starts with an RTL character, it looks fine).
 
Screenshot_20171016-172021.png
152 KB View Download
Screenshot_20171016-172025.png
155 KB View Download

Comment 1 by timloh@chromium.org, Oct 16 2017

Components: UI>Internationalization>RTL
Project Member

Comment 2 by bugdroid1@chromium.org, Oct 18 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/c42dbc153f640bc389f713b1d6001cec5d9354e8

commit c42dbc153f640bc389f713b1d6001cec5d9354e8
Author: Timothy Loh <timloh@chromium.org>
Date: Wed Oct 18 00:58:28 2017

Display modal permission prompts in RTL locales correctly.

This patch fixes modal permission prompts so that in RTL:
- The text direction is RTL as per the locale, instead of following the
  first strong directional character (typically the first character of
  the URL)
- The icon occurs on the right side of the dialog (i.e. before it)

Bug:  774926 
Change-Id: I8677cf61c30bdce95113e0e414531c343ed302c2
Reviewed-on: https://chromium-review.googlesource.com/722382
Reviewed-by: Bernhard Bauer <bauerb@chromium.org>
Commit-Queue: Timothy Loh <timloh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#509637}
[modify] https://crrev.com/c42dbc153f640bc389f713b1d6001cec5d9354e8/chrome/android/java/res/layout/permission_dialog.xml
[modify] https://crrev.com/c42dbc153f640bc389f713b1d6001cec5d9354e8/chrome/android/java/src/org/chromium/chrome/browser/permissions/PermissionDialogController.java

Comment 3 by timloh@chromium.org, Oct 23 2017

Labels: Merge-Request-63
Requesting a merge to M63 -- Modal permissions are enabled from M63, so it would be nice to not break permission UI for RTL users.

Comment 4 by cma...@chromium.org, Oct 24 2017

Is this a regression in M63? Have the fix been verified in Canary?

Comment 5 by timloh@chromium.org, Oct 24 2017

M63 changes the UI surface (launch bug 760360), and the new UI regresses prompts for RTL users. I just verified the fix in Canary.
Screenshot_20171024-124542.png
152 KB View Download
Project Member

Comment 6 by sheriffbot@chromium.org, Oct 24 2017

Labels: -Merge-Request-63 Hotlist-Merge-Approved Merge-Approved-63
Your change meets the bar and is auto-approved for M63. Please go ahead and merge the CL to branch 3239 manually. Please contact milestone owner if you have questions.
Owners: cmasso@(Android), cmasso@(iOS), gkihumba@(ChromeOS), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

Comment 7 by timloh@chromium.org, Oct 24 2017

Cc: cma...@chromium.org
cmasso@: I figure I should wait for your confirmation despite the auto-approval -- is this fine to merge?
Cc: krav...@chromium.org
kravula@, can you check the fix in M64 please?
Verified in M64-64.0.3250.0 build
Screenshot_٢٠١٧-١٠-٢٦-١١-٥٩-٠٨.png
132 KB View Download
Project Member

Comment 10 by sheriffbot@chromium.org, Oct 27 2017

This issue has been approved for a merge. Please merge the fix to any appropriate branches as soon as possible!

If all merges have been completed, please remove any remaining Merge-Approved labels from this issue.

Thanks for your time! To disable nags, add the Disable-Nags label.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Project Member

Comment 11 by bugdroid1@chromium.org, Oct 30 2017

Labels: -merge-approved-63 merge-merged-3239
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/7130688652b13a4f77434da1008ac68ab6283d37

commit 7130688652b13a4f77434da1008ac68ab6283d37
Author: Timothy Loh <timloh@chromium.org>
Date: Mon Oct 30 02:06:33 2017

Display modal permission prompts in RTL locales correctly.

This patch fixes modal permission prompts so that in RTL:
- The text direction is RTL as per the locale, instead of following the
  first strong directional character (typically the first character of
  the URL)
- The icon occurs on the right side of the dialog (i.e. before it)

Bug:  774926 
Change-Id: I8677cf61c30bdce95113e0e414531c343ed302c2
Reviewed-on: https://chromium-review.googlesource.com/722382
Reviewed-by: Bernhard Bauer <bauerb@chromium.org>
Commit-Queue: Timothy Loh <timloh@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#509637}(cherry picked from commit c42dbc153f640bc389f713b1d6001cec5d9354e8)
Reviewed-on: https://chromium-review.googlesource.com/742723
Reviewed-by: Timothy Loh <timloh@chromium.org>
Cr-Commit-Position: refs/branch-heads/3239@{#287}
Cr-Branched-From: adb61db19020ed8ecee5e91b1a0ea4c924ae2988-refs/heads/master@{#508578}
[modify] https://crrev.com/7130688652b13a4f77434da1008ac68ab6283d37/chrome/android/java/res/layout/permission_dialog.xml
[modify] https://crrev.com/7130688652b13a4f77434da1008ac68ab6283d37/chrome/android/java/src/org/chromium/chrome/browser/permissions/PermissionDialogController.java

Status: Fixed (was: Assigned)
Status: Verified (was: Fixed)
Verified in M63-63.0.3239.31

Sign in to add a comment