Issue metadata
Sign in to add a comment
|
Entire URL is struck thru for invalid HTTPS on iOS 11 |
||||||||||||||||||||||||
Issue description1) 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.
,
Aug 2 2017
,
Aug 2 2017
,
Aug 2 2017
,
Aug 2 2017
,
Aug 2 2017
Which Chrome versions does this issue appear on?
,
Aug 2 2017
And just as importantly, which iOS versions?
,
Aug 2 2017
lgarron@ Appears to be iOS11 only. I'm able to reproduce on M59/60/61/62.
,
Aug 2 2017
Great, sounds like you're on it! :-) I presume you're aware of elawrence@'s recent strikethrough-related changes. Let us know if not.
,
Aug 2 2017
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.
,
Aug 2 2017
,
Aug 2 2017
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?
,
Aug 3 2017
,
Aug 3 2017
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.
,
Aug 8 2017
Still reproduced with iOS11 beta#5 (15A5341f)
,
Aug 10 2017
,
Aug 10 2017
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!
,
Aug 11 2017
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
,
Aug 11 2017
Also worth noting: playing with defaultTextAttributes doesn't seem to help either.
,
Aug 11 2017
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?
,
Aug 11 2017
I have updated the radar with a new repro and stressed that this is again a regression.
,
Aug 11 2017
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
,
Aug 12 2017
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."
,
Aug 16 2017
Since Apple closed the previous radar, filed new radar 33914429 with this regression.
,
Aug 16 2017
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
,
Aug 17 2017
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?
,
Aug 17 2017
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.
,
Aug 17 2017
,
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
,
Aug 21 2017
The workaround has landed, I'm keeping this open for now to watch for possible fix in UIKit.
,
Aug 21 2017
Sounds good!
,
Aug 21 2017
Beta#7 update: Still reproduced on iOS11 beta#7 (15A5362a) Tested on M60 stable.
,
Aug 25 2017
Has there been any new iOS beta to test this again Srikanth?
,
Aug 25 2017
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.
,
Aug 28 2017
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.
,
Aug 28 2017
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
,
Aug 28 2017
Beta update: Bug still exists on iOS11 beta#8 M60 Stable.
,
Aug 28 2017
,
Aug 29 2017
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
,
Aug 29 2017
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.
,
Aug 31 2017
,
Aug 31 2017
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 |
|||||||||||||||||||||||||
Comment 1 by justincohen@chromium.org
, Aug 2 2017