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

Issue 751801 link

Starred by 1 user

Issue metadata

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



Sign in to add a comment

Entire URL is struck thru for invalid HTTPS on iOS 11

Project Member Reported by justincohen@chromium.org, Aug 2 2017

Issue description

1) Visit https://ssl-cert.org
2) Tap advanced and proceed (you should see 'Waa?')
3) Open a new tab and visit https://ssl-cert.org again.
4) url is struck thru.


 
Labels: ReleaseBlock-Stable Hotlist-iOS11 M-61
Labels: -Restrict-View-Google
Cc: bl...@blairschumann.com
Reported by blair@blairschumann.com
Components: UI>Browser>Omnibox>SecurityIndicators
Cc: elawrence@chromium.org lgar...@chromium.org
Which Chrome versions does this issue appear on?
And just as importantly, which iOS versions?
lgarron@ Appears to be iOS11 only.  I'm able to reproduce on M59/60/61/62.
Great, sounds like you're on it! :-)

I presume you're aware of elawrence@'s recent strikethrough-related changes. Let us know if not.
This may well be related to Apple's attempt to fix  crbug.com/711367 , an iOS 10.3 regression whereby they broke handling of addAttribute:NSStrikethroughStyleAttributeName. 

Google's open case with Apple is Radar #32909369.
Cc: -lgar...@chromium.org
Cc: justincohen@chromium.org
Owner: elawrence@chromium.org
Seems like this is indeed similar.  Here's what we are setting:

https{
    NSBaselineOffset = 0;
    NSColor = "UIExtendedSRGBColorSpace 0.772549 0.223529 0.160784 1";
    NSFont = "<UICTFont: 0x7fefc5c12850> font-family: \"Roboto-Regular\"; font-weight: normal; font-style: normal; font-size: 16.00pt";
    NSParagraphStyle = "Alignment 4, LineSpacing 0, ParagraphSpacing 0, ParagraphSpacingBefore 0, HeadIndent 0, TailIndent 0, FirstLineHeadIndent 0, LineHeight 0/0, LineHeightMultiple 0, LineBreakMode 2, Tabs (\n    28L,\n    56L,\n    84L,\n    112L,\n    140L,\n    168L,\n    196L,\n    224L,\n    252L,\n    280L,\n    308L,\n    336L\n), DefaultTabInterval 0, Blocks (\n), Lists (\n), BaseWritingDirection 0, HyphenationFactor 0, TighteningForTruncation NO, HeaderLevel 0";
    NSStrikethrough = 1;
}://{
    NSBaselineOffset = 0;
    NSColor = "UIExtendedGrayColorSpace 0.631373 1";
    NSFont = "<UICTFont: 0x7fefc5c12850> font-family: \"Roboto-Regular\"; font-weight: normal; font-style: normal; font-size: 16.00pt";
    NSParagraphStyle = "Alignment 4, LineSpacing 0, ParagraphSpacing 0, ParagraphSpacingBefore 0, HeadIndent 0, TailIndent 0, FirstLineHeadIndent 0, LineHeight 0/0, LineHeightMultiple 0, LineBreakMode 2, Tabs (\n    28L,\n    56L,\n    84L,\n    112L,\n    140L,\n    168L,\n    196L,\n    224L,\n    252L,\n    280L,\n    308L,\n    336L\n), DefaultTabInterval 0, Blocks (\n), Lists (\n), BaseWritingDirection 0, HyphenationFactor 0, TighteningForTruncation NO, HeaderLevel 0";
}ssl-cert.org{
    NSBaselineOffset = 0;
    NSColor = "UIExtendedGrayColorSpace 0 0.87";
    NSFont = "<UICTFont: 0x7fefc5c12850> font-family: \"Roboto-Regular\"; font-weight: normal; font-style: normal; font-size: 16.00pt";
    NSParagraphStyle = "Alignment 4, LineSpacing 0, ParagraphSpacing 0, ParagraphSpacingBefore 0, HeadIndent 0, TailIndent 0, FirstLineHeadIndent 0, LineHeight 0/0, LineHeightMultiple 0, LineBreakMode 2, Tabs (\n    28L,\n    56L,\n    84L,\n    112L,\n    140L,\n    168L,\n    196L,\n    224L,\n    252L,\n    280L,\n    308L,\n    336L\n), DefaultTabInterval 0, Blocks (\n), Lists (\n), BaseWritingDirection 0, HyphenationFactor 0, TighteningForTruncation NO, HeaderLevel 0";
}

Checking the UITextField a few moments (but not immediately) after setting the attributedText I see:

https{
    NSBaselineOffset = 0;
    NSColor = "UIExtendedSRGBColorSpace 0.772549 0.223529 0.160784 1";
    NSFont = "<UICTFont: 0x7fefc5c12850> font-family: \"Roboto-Regular\"; font-weight: normal; font-style: normal; font-size: 16.00pt";
    NSParagraphStyle = "Alignment 4, LineSpacing 0, ParagraphSpacing 0, ParagraphSpacingBefore 0, HeadIndent 0, TailIndent 0, FirstLineHeadIndent 0, LineHeight 0/0, LineHeightMultiple 0, LineBreakMode 4, Tabs (\n    28L,\n    56L,\n    84L,\n    112L,\n    140L,\n    168L,\n    196L,\n    224L,\n    252L,\n    280L,\n    308L,\n    336L\n), DefaultTabInterval 0, Blocks (\n), Lists (\n), BaseWritingDirection 0, HyphenationFactor 0, TighteningForTruncation NO, HeaderLevel 0";
    NSShadow = "NSShadow {0, -1} color = {(null)}";
    NSStrikethrough = 1;
}://{
    NSBaselineOffset = 0;
    NSColor = "UIExtendedGrayColorSpace 0.631373 1";
    NSFont = "<UICTFont: 0x7fefc5c12850> font-family: \"Roboto-Regular\"; font-weight: normal; font-style: normal; font-size: 16.00pt";
    NSParagraphStyle = "Alignment 4, LineSpacing 0, ParagraphSpacing 0, ParagraphSpacingBefore 0, HeadIndent 0, TailIndent 0, FirstLineHeadIndent 0, LineHeight 0/0, LineHeightMultiple 0, LineBreakMode 4, Tabs (\n    28L,\n    56L,\n    84L,\n    112L,\n    140L,\n    168L,\n    196L,\n    224L,\n    252L,\n    280L,\n    308L,\n    336L\n), DefaultTabInterval 0, Blocks (\n), Lists (\n), BaseWritingDirection 0, HyphenationFactor 0, TighteningForTruncation NO, HeaderLevel 0";
    NSShadow = "NSShadow {0, -1} color = {(null)}";
    NSStrikethrough = 1;
}ssl-cert.org{
    NSBaselineOffset = 0;
    NSColor = "UIExtendedGrayColorSpace 0 0.87";
    NSFont = "<UICTFont: 0x7fefc5c12850> font-family: \"Roboto-Regular\"; font-weight: normal; font-style: normal; font-size: 16.00pt";
    NSParagraphStyle = "Alignment 4, LineSpacing 0, ParagraphSpacing 0, ParagraphSpacingBefore 0, HeadIndent 0, TailIndent 0, FirstLineHeadIndent 0, LineHeight 0/0, LineHeightMultiple 0, LineBreakMode 4, Tabs (\n    28L,\n    56L,\n    84L,\n    112L,\n    140L,\n    168L,\n    196L,\n    224L,\n    252L,\n    280L,\n    308L,\n    336L\n), DefaultTabInterval 0, Blocks (\n), Lists (\n), BaseWritingDirection 0, HyphenationFactor 0, TighteningForTruncation NO, HeaderLevel 0";
    NSShadow = "NSShadow {0, -1} color = {(null)}";
    NSStrikethrough = 1;
}

What's the fix?  Should we need to set `NSStrikethrough = 0;` for the next segment?


Cc: linds...@chromium.org
Owner: ----
Status: Available (was: Assigned)
I can't really work on iOS right now (no devices, no build environment). It's not clear that Chrome needs a change here as the underlying regression is in the OS.
Still reproduced with iOS11 beta#5 (15A5341f)
Owner: stkhapugin@chromium.org
Status: Started (was: Available)
I've tried setting NSUnderlineStyleNone on the rest of the string, and it helps, but there are more issues (https://chromium-review.googlesource.com/c/610046 PS 1). 

1. repeat steps from the description
2a. observe that the URL is not strike through anymore, except https (with the patch) OR 
2b. observe that the URL is all crossed-out. 
3. start typing into omnibox
4. observe that whatever is being typed into omnibox is still crossed out!
Further tinkering revealed more detail:

1. It is easy to repro in a clean project with just a textfield. 
2. Once a textfield has a crossed-out symbol as the first character, it enters this bad state where all new text is crossed-out
3. Possible workaround that seems to work in an empty project: override UITextFieldDelegate's textField:shouldChangeCharactersInRange:replacementString: to always return NO and set attributed text manually from this callback. 
4. Possible workaround: not cross out the first symbol, i.e. insert a whitespace character or have the crossing line start after "h" at the first "t"
5. Potential way to solve this - move the scheme into a separate label

(3) is hard to implement in OmniboxTextfieldIOS because the textfield often has no text at all and overall very complex state management in this class
(4) is easy to implement with the "h" not crossed-out, but sticking a whitespace character in is hard
(5) is terrible and means a full rewrite of omnibox textfield on iOS
Also worth noting: playing with defaultTextAttributes doesn't seem to help either. 
stk@ did you update radar 32909369 to let them know strikethru has regressed worse, and attach this project?  This is "Bug ID 32909369: Regression: NSStrikethroughStyleAttribute not effective for subranges of a string"

There's usually only one more beta, so if not, sooner is better?
I have updated the radar with a new repro and stressed that this is again a regression. 
Re #21: Thanks!

Re #17: One thing to keep in mind is that in Chrome it's possible that the scheme isn't at the front of the string. I don't know whether view-source is supported at iOS, but if it is, here's a repro link: 

    view-source:https://expired.badssl.com
Per https://bugreport.apple.com/web/?problemID=32909369:
 "Because the original issue is resolved, and now there is a new regression, we need you to file a new bug report."
Since Apple closed the previous radar, filed new radar 33914429 with this regression. 
Using a zero-width whitespace character as a prefix to the string set in UITextField seems to work around this:
https://chromium-review.googlesource.com/c/616772
Labels: Needs-Feedback
Status: Assigned (was: Started)
Here's a CL to remove strikethrough from h: https://chromium-review.googlesource.com/c/618870
I attach a screenshot of how it looks in this case. This is by far the lowest risk solution in terms of other omnibox regressions. 

The solution with zero-width whitespace can potentially interfere with copy-paste of the prefixed URL or with autocomplete suggestions, even though it seems to be fine from the first look.

What do you think? 
ttps.png
36.7 KB View Download
Summary: Entire URL is struck thru for invalid HTTPS on iOS 11 (was: Entire URL is struck thru)
Recent analysis from the UX research team suggests that the strikethrough results in a statistically significant improvement in users' understanding of the non-security of the page, so we'll definitely want to keep it.

Skipping strikethrough for the first character isn't great, but I agree that it's probably a better short-term choice than trying hackery with zero-width characters. If it turns out that Apple never plans to fix this in iOS at all, we should consider investing in more elaborate workarounds.
Labels: -Needs-Feedback
Project Member

Comment 29 by bugdroid1@chromium.org, Aug 21 2017

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

commit b927dc7507b2d9bc23a2aa36951afacc41df8a9c
Author: stkhapugin@chromium.org <stkhapugin@chromium.org>
Date: Mon Aug 21 10:02:25 2017

Workaround for UITextField bug in iOS11 with strikethrough.

Introduces a workaround for the strikethrough bug in UITextField where
the first character never has strikethrough.
The workaround is to omit the strikethrough on the first character.
This leads to 'h' in "https" being visibly not struck through, which is 
not perfect, but allows us to keep the strikethrough on the rest of the
string. A better long-term fix needs to be found should the textfield 
keep this bug in a future public iOS release. 

Bug:  751801 
Change-Id: Ibf18dac7b323a5746ac78e85260dac5efa2ace16
Reviewed-on: https://chromium-review.googlesource.com/618870
Commit-Queue: Stepan Khapugin <stkhapugin@chromium.org>
Reviewed-by: Rohit Rao (ping after 24h) <rohitrao@chromium.org>
Cr-Commit-Position: refs/heads/master@{#495898}
[modify] https://crrev.com/b927dc7507b2d9bc23a2aa36951afacc41df8a9c/ios/chrome/browser/ui/omnibox/omnibox_view_ios.mm

The workaround has landed, I'm keeping this open for now to watch for possible fix in UIKit. 
Sounds good!
Beta#7 update: Still reproduced on iOS11 beta#7 (15A5362a)
Tested on M60 stable.
Has there been any new iOS beta to test this again Srikanth?
Fix from comment#29 is verified on M62.0.3196.0 canary with iOS11 beta7.
reply#33: No new beta yet. I will re-test if there is any #beta8 next week.
Labels: Merge-Request-61
Requesting cherry-pick of the workaround from comment#29 to M61. Keeping the bug open since we still need to come up with a good solution in case this is not fixed in iOS 11 GM. 
Project Member

Comment 36 by sheriffbot@chromium.org, Aug 28 2017

Labels: -Merge-Request-61 Merge-Review-61 Hotlist-Merge-Review
This bug requires manual review: Less than 4 days to go before AppStore submit on M61
Please contact the milestone owner if you have questions.
Owners: amineer@(Android), cmasso@(iOS), ketakid@(ChromeOS), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Beta update: Bug still exists on iOS11 beta#8 M60 Stable.
Labels: -Hotlist-Merge-Review -Merge-Review-61 Merge-Approved-61
Project Member

Comment 39 by bugdroid1@chromium.org, Aug 29 2017

Labels: -merge-approved-61 merge-merged-3163
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/0e0508e58da681b1ab7e89928fb85f3ad1bd97de

commit 0e0508e58da681b1ab7e89928fb85f3ad1bd97de
Author: stkhapugin@chromium.org <stkhapugin@chromium.org>
Date: Tue Aug 29 11:01:22 2017

Workaround for UITextField bug in iOS11 with strikethrough.

Introduces a workaround for the strikethrough bug in UITextField where
the first character never has strikethrough.
The workaround is to omit the strikethrough on the first character.
This leads to 'h' in "https" being visibly not struck through, which is
not perfect, but allows us to keep the strikethrough on the rest of the
string. A better long-term fix needs to be found should the textfield
keep this bug in a future public iOS release.

TBR=stkhapugin@chromium.org

(cherry picked from commit b927dc7507b2d9bc23a2aa36951afacc41df8a9c)

Bug:  751801 
Change-Id: Ibf18dac7b323a5746ac78e85260dac5efa2ace16
Reviewed-on: https://chromium-review.googlesource.com/618870
Commit-Queue: Stepan Khapugin <stkhapugin@chromium.org>
Reviewed-by: Rohit Rao (ping after 24h) <rohitrao@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#495898}
Reviewed-on: https://chromium-review.googlesource.com/640706
Reviewed-by: Stepan Khapugin <stkhapugin@chromium.org>
Cr-Commit-Position: refs/branch-heads/3163@{#968}
Cr-Branched-From: ff259bab28b35d242e10186cd63af7ed404fae0d-refs/heads/master@{#488528}
[modify] https://crrev.com/0e0508e58da681b1ab7e89928fb85f3ad1bd97de/ios/chrome/browser/ui/omnibox/omnibox_view_ios.mm

We need to close this bug and create a new one for tracking purpose since this one is fixed for M61 and can no longer be blocking.
Status: Fixed (was: Assigned)
Filed  crbug.com/760964  and closing this one. 
Status: Verified (was: Fixed)
Verified on 61.0.3163.73 Beta on iPhone 6plus(iOS 11 beta8) and iPad Air(iOS 11 beta8).

Working as mentioned in comment #29, h in https being visible as not struck through rest of the string is getting strikethrough.

Link to screenshot: 
https://drive.google.com/a/google.com/file/d/0B8Cek8RsDbF8Q0hwLTlyaEJLckU/view?usp=sharing

Since this issue requires permanent solution a new bug  crbug.com/760964  is created for tracking and changing this bug to verified.

Sign in to add a comment