| Summary: | REGRESSION (r245672): <select> dropdown with text-rendering: optimizeLegibility freezes Safari | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | yann | ||||||||||
| Component: | Text | Assignee: | Myles C. Maxfield <mmaxfield> | ||||||||||
| Status: | RESOLVED FIXED | ||||||||||||
| Severity: | Critical | CC: | achristensen, andrea, ap, benjamin, bunnyhero, cdumez, cmarcelo, commit-queue, darin, dbates, dino, ews-watchlist, jonlee, karolis.n, matthewwithanm, mmaxfield, nilfgard, simon.fraser, thorton, webkit-bug-importer, yann | ||||||||||
| Priority: | P2 | Keywords: | InRadar | ||||||||||
| Version: | Other | ||||||||||||
| Hardware: | Mac | ||||||||||||
| OS: | macOS 10.14 | ||||||||||||
| See Also: |
https://bugs.webkit.org/show_bug.cgi?id=202556 https://bugs.webkit.org/show_bug.cgi?id=202262 https://bugs.webkit.org/show_bug.cgi?id=204138 |
||||||||||||
| Attachments: |
|
||||||||||||
I cannot reproduce this. Could you please attach a complete crash log? 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...). Thank you! rdar://problem/55744833 *** Bug 202262 has been marked as a duplicate of this bug. *** I can reproduce this. 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. The font descriptor is identical on Mojave and Catalina:
NSCTFontDescriptor <0x60000260bd80> = {
CTFontDescriptorLanguageAttribute = "";
NSCTFontCascadeListAttribute = (
);
NSCTFontOpticalSizeAttribute = auto;
NSCTFontUIUsageAttribute = CTFontRegularUsage;
NSFontSizeAttribute = 11;
}
NSCTFontOpticalSizeAttribute is the field that's tripping up NSFont. Created attachment 379981 [details]
Patch
Created attachment 379982 [details]
Patch
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. Created attachment 380042 [details]
Patch for committing
Created attachment 380063 [details]
Patch for committing
Comment on attachment 380063 [details] Patch for committing Clearing flags on attachment: 380063 Committed r250640: <https://trac.webkit.org/changeset/250640> Does this fix address the dupe (bug 202262)? (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 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? 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. 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]; 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). 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. 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. 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. *** Bug 202055 has been marked as a duplicate of this bug. *** |
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>