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

Issue 735759 link

Starred by 3 users

Issue metadata

Status: Fixed
Owner:
Closed: Jul 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 1
Type: Bug
Proj-VR
Proj-XR
Proj-XR-VR



Sign in to add a comment

HTTPS scheme strikethrough is nearly invisible in Incognito mode

Project Member Reported by est...@chromium.org, Jun 22 2017

Issue description

Chrome 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.
 

Comment 1 by est...@chromium.org, Jun 22 2017

Summary: HTTPS scheme strikethrough is nearly invisible in Incognito mode (was: HTTPS scheme strikethrough is nearly invisible)
Owner: cjgrant@chromium.org
Status: Assigned (was: Untriaged)
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).




Owner: est...@chromium.org
Over to Emily to direct how we address the visibility problem (please see my suggestions).
Clank's strike line.
clank-incognito-https.png
25.0 KB View Download

Comment 5 by est...@chromium.org, Jun 22 2017

Cc: -cjgrant@chromium.org maxwalker@chromium.org est...@chromium.org
Owner: cjgrant@chromium.org
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.
Attaching a VR screenshot of the equivalent.
temp.png
268 KB View Download
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).
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.
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.
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:




temp.png
260 KB View Download
And, double-thickness with matching scheme and host color:
temp.png
272 KB View Download
Cc: martinshelton@chromium.org
Cc: joshcarpenter@chromium.org
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.
Peter, here's white scheme, single-thickness strikethrough:


temp.png
270 KB View Download
>> 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.
Options.png
177 KB View Download
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).
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.
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.
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.
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.
Status: Started (was: Assigned)
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/
Project Member

Comment 24 by bugdroid1@chromium.org, 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

Status: Fixed (was: Started)
Labels: Test-Manual

Sign in to add a comment