HTTPS scheme strikethrough is nearly invisible in Incognito mode |
|||||||||
Issue descriptionChrome Version: 61.0.3137.0 OS: Android What steps will reproduce the problem? (1) Visit https://expired.badssl.com in incognito in VR mode. What is the expected result? The https:// is visibly styled in strikethrough. What happens instead? The strikethrough is nearly invisible, very hard to see. We have recent research showing that the strikethrough significantly improves user comprehension, so I think it's important to fix this for M61.
,
Jun 22 2017
I completely agree that this is an issue, albeit slightly ironic. Linux desktop maintains its red https coloring even when incognito. Clank incognito loses the red, and shows a grey https with a very thin horizontal line, which IMO is about as visible as the VR version. It makes sense to do better. Emily, which (one or more) of these options would you like to see: - Keep the red coloring on icon and scheme (and possibly also green on secure pages) - Incorporate a thicker horizontal line via gfx::RenderText. Lucas removed the diagonal-strike code, but maybe it could be reintroduced as "heavy-strike" styling. Currently, RenderText appears to hard-code the line thickness as 1/18th of text size. - Draw our own thicker strike-through over the text, using a rectangle and text calculations (not entirely crazy).
,
Jun 22 2017
Over to Emily to direct how we address the visibility problem (please see my suggestions).
,
Jun 22 2017
Clank's strike line.
,
Jun 22 2017
We keep the coloring on Linux desktop because the omnibox background is white, but on Clank (VR + normal Clank) where the background is gray, we don't attempt to draw colors. Can you please send screenshots of Clank and VR mode to maxwalker@ and ask him what he thinks and if he has any suggestions? May be something we should fix on both platforms.
,
Jun 26 2017
Attaching a VR screenshot of the equivalent.
,
Jun 26 2017
My simple suggestion would be to make the line brighter and/or make it thicker, but it sounds like we can't do this (per your email comment re: gfx::RenderText).
,
Jun 26 2017
Not out of the box, no. However, the ratio of text height to strike-through height is hard-coded, and gfx:: folks might let us add a setter to override the default value (in lieu of specifying a weight per-range). I'll look into this, but the question still remains as to whether that's the preferred solution.
,
Jun 26 2017
This was debated a while back and the correct answer is that in incognito mode the background should be dark gray and the scheme and strikethrough should be the same white as the hostname. It sounds like both Android and Linux get parts of this wrong. Windows is correct, copy that.
,
Jun 26 2017
Interestingly, when Emily reported this bug, the scheme was actually emphasized like the host, because we were following desktop. A bug was filed to make it match Clank, and my last screenshot has the fix. So, even if we return to "white" scheme and hostname, we're back to something Emily considers hard-to-see (and it certainly is). Peter, what do you think about thickening the line in VR? Here's a shot with the line thickness doubled:
,
Jun 26 2017
And, double-thickness with matching scheme and host color:
,
Jun 26 2017
,
Jun 26 2017
,
Jun 26 2017
I don't think doubling the thickness helps when you're still matching Android's buggy coloring scheme. (P.S. if there isn't one already, please file a bug against Android to fix their behavior.) Can you show me a screenshot of restoring the Windows behavior? That is, white scheme, single-thickness strikethrough. It's hard for me to know whether double thickness is much different when that's my only comparison point. It would also be nice to understand what sorts of factors about the VR interface (e.g. lower pixel density?) would govern how we tweak it compared to desktop. If we find something hard-to-notice in VR, I would immediately ask whether it is hard-to-notice on desktop, and if not, why there is a difference.
,
Jun 26 2017
Peter, here's white scheme, single-thickness strikethrough:
,
Jun 27 2017
>> The strikethrough is nearly invisible, very hard to see. Using a bright strikethrough on a dimmed scheme seems like a reasonable approach to me (if technically possible with gfx::RenderText). This would make the strikethrough and icon stand out more. See quick mock for VR attached. WDYT? I tried applying a light red color to the scheme, however we would need to darken the omnibox background to get an acceptable contrast which would introduce a lot of additional complexity.
,
Jun 27 2017
Thanks Max! Personally I like option 4 (the red). I think it reflects "dangerous" well, and in general, I don't see why incognito would want to mask this information in any way. Option 3, the differently-colored strike line, isn't possible with RenderText (that I can see). We could potentially hack the rendering to draw our own line, but I'd really prefer to stay within the API (this is why I pinged gfx:: to see if we can open the line thickness setting). The ability to configure line thickness seems easy to add. The ability to configure a different colored line (relative to text) would be much trickier I believe (as we'd now need to set multiple colors across runs of characters).
,
Jun 28 2017
Strong +1 to double-thickness and same color, and we can iterate as needed in M62 and beyond. For context, this release of Chrome for VR is going to be akin to a 0.1 and seen by very few users (e.g. will not show up in Daydream Home). Across the board we're targeting polish and functionality far below our targeted 1.0 release. I think we want to target "good enough" here, as elsewhere, then improve iteratively in subsequent releases.
,
Jun 28 2017
Changing the color of the scheme in incognito mode away from white has been vetoed (repeatedly) by the security/UX folks in the past. Don't do that. (Honestly, I know this is unsatisfying, but I don't even want to relitigate it after discussing the whole thing twice in the past. I didn't agree with the decision, but it's made, and I don't want to fight it a third time.) Making the scheme grey and the strikethrough white differs from every other platform, and RenderText can't currently do it anyway. I wouldn't go down this road. It might be worth trying to ram through for all platforms (not just VR) some day in the future. I would make the strikethrough double thickness and move on.
,
Jun 28 2017
Got it - thanks, Christopher and Josh! This option (white scheme, double-thickness strikethrough) SGTM, too. I'll keep you in the loop with any changes we might do in the future.
,
Jun 28 2017
Excellent. Thanks everyone for quickly closing on this, and Peter, thanks also for the historical perspective. Separately, I will file the bug Peter requested to make Clank also use a white scheme in this case.
,
Jun 30 2017
,
Jun 30 2017
Update: There's a CL posted here. I have reservations about how the custom line thickness is being plumbed through, and have asked msw@ for review. https://codereview.chromium.org/2969623004/
,
Jul 6 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/6c111c024ce31e4de508032dc8b7a18564007ed5 commit 6c111c024ce31e4de508032dc8b7a18564007ed5 Author: cjgrant <cjgrant@chromium.org> Date: Thu Jul 06 14:28:21 2017 RenderText: Allow strike-through line thickness to be customized. This CL plumbs in the ability to override the default strike-through line thickness used by RenderText. Along the way: - Remove unused underscore thickness configuration hooks. - Remove unncessary DrawDecorations wrapper. BUG= 735759 Review-Url: https://codereview.chromium.org/2969623004 Cr-Commit-Position: refs/heads/master@{#484580} [modify] https://crrev.com/6c111c024ce31e4de508032dc8b7a18564007ed5/chrome/browser/android/vr_shell/textures/render_text_wrapper.cc [modify] https://crrev.com/6c111c024ce31e4de508032dc8b7a18564007ed5/chrome/browser/android/vr_shell/textures/render_text_wrapper.h [modify] https://crrev.com/6c111c024ce31e4de508032dc8b7a18564007ed5/chrome/browser/android/vr_shell/textures/url_bar_texture.cc [modify] https://crrev.com/6c111c024ce31e4de508032dc8b7a18564007ed5/chrome/browser/android/vr_shell/textures/url_bar_texture_unittest.cc [modify] https://crrev.com/6c111c024ce31e4de508032dc8b7a18564007ed5/ui/gfx/render_text.cc [modify] https://crrev.com/6c111c024ce31e4de508032dc8b7a18564007ed5/ui/gfx/render_text.h [modify] https://crrev.com/6c111c024ce31e4de508032dc8b7a18564007ed5/ui/gfx/render_text_harfbuzz.cc [modify] https://crrev.com/6c111c024ce31e4de508032dc8b7a18564007ed5/ui/gfx/render_text_mac.mm [modify] https://crrev.com/6c111c024ce31e4de508032dc8b7a18564007ed5/ui/gfx/render_text_unittest.cc
,
Jul 6 2017
,
Feb 7 2018
|
|||||||||
►
Sign in to add a comment |
|||||||||
Comment 1 by est...@chromium.org
, Jun 22 2017