Issue metadata
Sign in to add a comment
|
Permission message text cannot have trailing period on Android |
||||||||||||||||||||||||
Issue descriptionToday, on Android, even if the permission message text in the .grd file has a trailing period, it's automatically removed and not showing up. For an example see [1]. It turns out that we do this explicitly at [2] (special thanks to dominickn@ for pointing out this). With the following TODO: // TODO(timloh): Currently the strings are shared with infobars, so we for now manually remove the full stop (this code catches most but not all languages). Update the strings after removing the infobar path. From dominickn@, "the stripping was only there because of infobars, and we no longer use infobars for permissions (we were meant to delete that code but...)" It's unclear why infobars do not allow trailing periods. Maybe it's a UX decision at that point. Proposal: fix the TODO and remove the trailing-period-stripping logic so that we always show the exact text as specified in the .grd file. This gives feature owners more flexibility of controlling the UX, and makes the code simpler to understand (least surprise). [1] https://chromium-review.googlesource.com/c/chromium/src/+/996769 [2] https://cs.chromium.org/chromium/src/chrome/android/java/src/org/chromium/chrome/browser/permissions/PermissionDialogView.java?rcl=a4b4b2d5ef79c36210ccd79c2d091ab78a27becb&l=89
,
Apr 12 2018
I'm also supportive of this change. The UX guideline is to omit the final period for a single sentence. However, I don't think this should be done programmatically (this bug is a good example of why not).
,
Apr 12 2018
amyroberts@, please chime in if you disagree.
,
Apr 12 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/dd201aa07beff63bd8be053e1657a514c08d7121 commit dd201aa07beff63bd8be053e1657a514c08d7121 Author: Xiaohan Wang <xhwang@chromium.org> Date: Thu Apr 12 18:59:44 2018 Allow trailing period in Android permission dialog message text Remove the logic that strips trailing period in the message text of Android permission dialog. Also add a trailing period in the message text of protected content permission prompt. Also remove a trailing period in the message text of notification permission prompt. Previously it was stripped pragmatically. Now remove it from the text to keep the same UX. See screenshot: https://photos.app.goo.gl/UKNWdos0ZmIPPuEE2 TBR=srahim@chromium.org Bug: 831881 Test: Manually checked the result Change-Id: I69ba2992214c438761262f51634c9b0c0d88c58e Reviewed-on: https://chromium-review.googlesource.com/1008995 Commit-Queue: Xiaohan Wang <xhwang@chromium.org> Reviewed-by: Dominick Ng <dominickn@chromium.org> Reviewed-by: Timothy Loh <timloh@chromium.org> Cr-Commit-Position: refs/heads/master@{#550291} [modify] https://crrev.com/dd201aa07beff63bd8be053e1657a514c08d7121/chrome/android/java/src/org/chromium/chrome/browser/permissions/PermissionDialogView.java [modify] https://crrev.com/dd201aa07beff63bd8be053e1657a514c08d7121/chrome/app/generated_resources.grd
,
Apr 13 2018
,
Apr 17 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/dd201aa07beff63bd8be053e1657a514c08d7121 commit dd201aa07beff63bd8be053e1657a514c08d7121 Author: Xiaohan Wang <xhwang@chromium.org> Date: Thu Apr 12 18:59:44 2018 Allow trailing period in Android permission dialog message text Remove the logic that strips trailing period in the message text of Android permission dialog. Also add a trailing period in the message text of protected content permission prompt. Also remove a trailing period in the message text of notification permission prompt. Previously it was stripped pragmatically. Now remove it from the text to keep the same UX. See screenshot: https://photos.app.goo.gl/UKNWdos0ZmIPPuEE2 TBR=srahim@chromium.org Bug: 831881 Test: Manually checked the result Change-Id: I69ba2992214c438761262f51634c9b0c0d88c58e Reviewed-on: https://chromium-review.googlesource.com/1008995 Commit-Queue: Xiaohan Wang <xhwang@chromium.org> Reviewed-by: Dominick Ng <dominickn@chromium.org> Reviewed-by: Timothy Loh <timloh@chromium.org> Cr-Commit-Position: refs/heads/master@{#550291} [modify] https://crrev.com/dd201aa07beff63bd8be053e1657a514c08d7121/chrome/android/java/src/org/chromium/chrome/browser/permissions/PermissionDialogView.java [modify] https://crrev.com/dd201aa07beff63bd8be053e1657a514c08d7121/chrome/app/generated_resources.grd |
|||||||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||||||
Comment 1 by dominickn@chromium.org
, Apr 12 2018