Issue metadata
Sign in to add a comment
|
Modal permission prompts don't work properly in RTL |
||||||||||||||||||||||||
Issue descriptionAttached 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).
,
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
,
Oct 23 2017
Requesting a merge to M63 -- Modal permissions are enabled from M63, so it would be nice to not break permission UI for RTL users.
,
Oct 24 2017
Is this a regression in M63? Have the fix been verified in Canary?
,
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.
,
Oct 24 2017
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
,
Oct 24 2017
cmasso@: I figure I should wait for your confirmation despite the auto-approval -- is this fine to merge?
,
Oct 26 2017
kravula@, can you check the fix in M64 please?
,
Oct 26 2017
Verified in M64-64.0.3250.0 build
,
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
,
Oct 30 2017
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
,
Oct 31 2017
,
Nov 1 2017
Verified in M63-63.0.3239.31 |
|||||||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||||||
Comment 1 by timloh@chromium.org
, Oct 16 2017