RESOLVED FIXED 202198
REGRESSION (r245672): <select> dropdown with text-rendering: optimizeLegibility freezes Safari 
https://bugs.webkit.org/show_bug.cgi?id=202198
Summary REGRESSION (r245672): <select> dropdown with text-rendering: optimizeLegibili...
yann
Reported 2019-09-25 04:47:35 PDT
Hi, Here is a test case: the dropdown has a custom appearance and inherits text-rendering: optimizeLegibility from the body. Click it to crash the page. <!doctype html> <html> <head> <style> body { text-rendering: optimizeLegibility; } select { border: 1px solid red; height: 32px; width: 200px; -webkit-appearance: none; /* text-rendering: auto; uncomment and it's fine */ } </style> </head> <body> <select> <option>click</option> <option>click</option> </select> </body> </html>
Attachments
Patch (6.92 KB, patch)
2019-10-01 19:18 PDT, Myles C. Maxfield 		no flags
DetailsFormatted DiffDiff
Patch (6.96 KB, patch)
2019-10-01 19:23 PDT, Myles C. Maxfield 		thorton: review+
DetailsFormatted DiffDiff
Patch for committing (6.98 KB, patch)
2019-10-02 11:30 PDT, Myles C. Maxfield 		no flags
DetailsFormatted DiffDiff
Patch for committing (6.98 KB, patch)
2019-10-02 15:10 PDT, Myles C. Maxfield 		no flags
DetailsFormatted DiffDiff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
Alexey Proskuryakov
Comment 1 2019-09-25 15:17:16 PDT
I cannot reproduce this. Could you please attach a complete crash log?
yann
Comment 2 2019-09-25 16:23:45 PDT
If that helps: this bug crashes Safari 13 (and 13.01) on both my Mac Mini (2012) and my Macbook (2014). Both run Mojave. How it happens: - I click the button, - the menu doesn't show up and the Safari interface is stuck. - After that, the beachball appears and the only thing I can do is to force quit Safari. I've sent you the report by email (I don't know if it contains sensitive informations. I doubt it, but just in case...).
Alexey Proskuryakov
Comment 3 2019-09-26 08:54:47 PDT
Thank you! rdar://problem/55744833
Alexey Proskuryakov
Comment 4 2019-09-29 15:22:05 PDT
*** Bug 202262 has been marked as a duplicate of this bug. ***
Myles C. Maxfield
Comment 5 2019-10-01 12:19:53 PDT
I can reproduce this.
Myles C. Maxfield
Comment 6 2019-10-01 12:50:07 PDT
Oh, it looks like [NSFont fontWithDescriptor:size:] throws an Objective-C exception: -[NSTaggedPointerString _getValue:forType:]: unrecognized selector We never surrounded this code with try/catch blocks, so we pop back up to the runloop, thereby confusing the UI process.
Myles C. Maxfield
Comment 7 2019-10-01 14:16:33 PDT
The font descriptor is identical on Mojave and Catalina: NSCTFontDescriptor <0x60000260bd80> = { CTFontDescriptorLanguageAttribute = ""; NSCTFontCascadeListAttribute = ( ); NSCTFontOpticalSizeAttribute = auto; NSCTFontUIUsageAttribute = CTFontRegularUsage; NSFontSizeAttribute = 11; }
Myles C. Maxfield
Comment 8 2019-10-01 15:15:48 PDT
NSCTFontOpticalSizeAttribute is the field that's tripping up NSFont.
Myles C. Maxfield
Comment 9 2019-10-01 15:19:45 PDT
Caused by r245672.
Myles C. Maxfield
Comment 10 2019-10-01 15:20:48 PDT
<rdar://problem/55744833>
Myles C. Maxfield
Comment 11 2019-10-01 19:18:54 PDT
Created attachment 379981 [details] Patch
Myles C. Maxfield
Comment 12 2019-10-01 19:23:27 PDT
Created attachment 379982 [details] Patch
Tim Horton
Comment 13 2019-10-01 19:58:38 PDT
Comment on attachment 379982 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=379982&action=review > Source/WTF/wtf/Platform.h:1602 > +#define USE_CORETEXT_AUTO_OPTICAL_SIZING_WORKAROUND 1 Pretty sure this is a HAVE() not a USE() (which is about using features of the platform, not internal workarounds) HAVE_CORETEXT_WITH_BROKEN_AUTO_OPTICAL_SIZING or vice versa or something like that > Source/WebKit/UIProcess/mac/WebPopupMenuProxyMac.mm:109 > + NSMutableDictionary *mutableDictionary = [(__bridge NSDictionary *)data.fontInfo.fontAttributeDictionary.get() mutableCopy]; Why not a RetainPtr & adoptNS? > Source/WebKit/UIProcess/mac/WebPopupMenuProxyMac.mm:115 > + [mutableDictionary removeObjectForKey:(NSString *)kCTFontOpticalSizeAttribute]; > + if (NSNumber *size = [mutableDictionary objectForKey:(NSString *)kCTFontSizeAttribute]) > + [mutableDictionary setObject:size forKey:(NSString *)kCTFontOpticalSizeAttribute]; Some might say if you're going to include one bridge you might as well bridge them all. It's up to you since it really doesn't matter yet.
Myles C. Maxfield
Comment 14 2019-10-02 11:30:53 PDT
Created attachment 380042 [details] Patch for committing
Myles C. Maxfield
Comment 15 2019-10-02 15:10:12 PDT
Created attachment 380063 [details] Patch for committing
WebKit Commit Bot
Comment 16 2019-10-02 17:53:38 PDT
Comment on attachment 380063 [details] Patch for committing Clearing flags on attachment: 380063 Committed r250640: <https://trac.webkit.org/changeset/250640>
Alexey Proskuryakov
Comment 17 2019-10-03 10:45:37 PDT
Does this fix address the dupe (bug 202262)?
Myles C. Maxfield
Comment 18 2019-10-03 18:33:26 PDT
(In reply to Alexey Proskuryakov from comment #17) > Does this fix address the dupe (bug 202262)? Nope! https://bugs.webkit.org/show_bug.cgi?id=202262
Darin Adler
Comment 19 2019-10-08 09:39:39 PDT
Comment on attachment 380063 [details] Patch for committing View in context: https://bugs.webkit.org/attachment.cgi?id=380063&action=review > Source/WebKit/UIProcess/mac/WebPopupMenuProxyMac.mm:124 > + END_BLOCK_OBJC_EXCEPTIONS If an exception is blocked here, font may be nil. Does the code below handle that?
Darin Adler
Comment 20 2019-10-08 09:40:14 PDT
Comment on attachment 380063 [details] Patch for committing View in context: https://bugs.webkit.org/attachment.cgi?id=380063&action=review >> Source/WebKit/UIProcess/mac/WebPopupMenuProxyMac.mm:124 >> + END_BLOCK_OBJC_EXCEPTIONS > > If an exception is blocked here, font may be nil. Does the code below handle that? I suggest rearranging the code so that font will be set to [NSFont menuFontOfSize:0] if there is an exception.
Darin Adler
Comment 21 2019-10-08 09:41:22 PDT
Comment on attachment 380063 [details] Patch for committing View in context: https://bugs.webkit.org/attachment.cgi?id=380063&action=review >>> Source/WebKit/UIProcess/mac/WebPopupMenuProxyMac.mm:124 >>> + END_BLOCK_OBJC_EXCEPTIONS >> >> If an exception is blocked here, font may be nil. Does the code below handle that? > > I suggest rearranging the code so that font will be set to [NSFont menuFontOfSize:0] if there is an exception. Like this: NSFont *font = nil; BEGIN_BLOCK_OBJC_EXCEPTIONS // complex code that is risky and can throw an exception END_BLOCK_OBJC_EXCEPTIONS if (!font) font = [NSFont menuFontOfSize:0];
Karolis
Comment 22 2019-10-17 06:06:13 PDT
I'm not sure which Safari version the patch is supposed to land in, but this issue still exists in Safari 13.0.2 on Mojave (early 2015 Mac). However, it works fine in Safari 13.0.2 on Catalina (2017 Mac).
Alexey Proskuryakov
Comment 23 2019-10-17 13:24:18 PDT
This should be fixed in an update that shipped yesterday. It still has the same 13.0.2 version as before, so we need to check build number to confirm. Could you please check build number via Safari About box? The one with the fix is 14608.2.40.1.3. If you have an older one, the new one can be installed via Software Update.
bunnyhero
Comment 24 2019-12-02 13:32:39 PST
It's still happening to me on Mojave 10.14.6 (18G103), Safari 13.0.3 (14608.3.10.10.1). This website crashes when I click the "Select Combo Tickets" dropdown: https://www.rom.on.ca/en/visit-us/tickets-hours If I add a custom stylesheet that contains `select { text-rendering: optimizeSpeed !important; }`, the crash does not occur.
Alexey Proskuryakov
Comment 25 2019-12-02 23:29:25 PST
Thank you, I can reproduce. This was fixed in Safari 13.0.2 (14608.2.40.1.3), and it's broken again in Safari 13.0.3. Keeping this bug in RESOLVED/FIXED state, because the fix is still present in latest WebKit code base. I started a discussion within Apple.
Sam Sneddon [:gsnedders]
Comment 26 2021-03-25 16:49:50 PDT
*** Bug 202055 has been marked as a duplicate of this bug. ***
Note You need to log in before you can comment on or make changes to this bug.