Remove the "Show Link Previews" and "Hide Link Previews" action menus in the preview platter
<rdar://55190038>
Created attachment 378948 [details] Patch
Created attachment 378949 [details] Patch
Comment on attachment 378949 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=378949&action=review > Source/WebKit/ChangeLog:5 > + https://bugs.webkit.org/show_bug.cgi?id=201864 > + Missing radar number. > Tools/TestWebKitAPI/Tests/WebKitCocoa/ContextMenus.mm:262 > + return [UIViewController new]; autorelease?
Committed r249950: <https://trac.webkit.org/changeset/249950>
(In reply to Dean Jackson from comment #5) > Committed r249950: <https://trac.webkit.org/changeset/249950> This seems to have broken the iOS build, as also indicated the EWS.
This seems to be the relevant build error: /Volumes/Data/worker/iOS-12-Build-EWS/build/Source/WebKit/UIProcess/API/Cocoa/_WKElementAction.mm:167:10: error: variable 'handler' is used uninitialized whenever switch case is taken [-Werror,-Wsometimes-uninitialized] case _WKElementActionToggleShowLinkPreviews: ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ /Volumes/Data/worker/iOS-12-Build-EWS/build/Source/WebKit/UIProcess/API/Cocoa/_WKElementAction.mm:175:92: note: uninitialized use occurs here return [[[self alloc] _initWithTitle:(customTitle ? customTitle : title) actionHandler:handler type:type assistant:assistant] autorelease]; ^~~~~~~ /Volumes/Data/worker/iOS-12-Build-EWS/build/Source/WebKit/UIProcess/API/Cocoa/_WKElementAction.mm:125:36: note: did you mean to use __block 'handler'? WKElementActionHandlerInternal handler; ^ __block 1 error generated.
Reverted r249950 for reason: Broke the iOS build. Committed r249956: <https://trac.webkit.org/changeset/249956>
Committed r250015: <https://trac.webkit.org/changeset/250015>
(In reply to Dean Jackson from comment #9) > Committed r250015: <https://trac.webkit.org/changeset/250015> The build fix was this: diff --git a/Source/WebKit/UIProcess/API/Cocoa/_WKElementAction.mm b/Source/WebKit/UIProcess/API/Cocoa/_WKElementAction. mm index 4b5ff1bec28..31ec821c9b7 100644 --- a/Source/WebKit/UIProcess/API/Cocoa/_WKElementAction.mm +++ b/Source/WebKit/UIProcess/API/Cocoa/_WKElementAction.mm @@ -121,8 +121,8 @@ + (instancetype)_elementActionWithType:(_WKElementActionType)type title:(NSStrin + (instancetype)_elementActionWithType:(_WKElementActionType)type customTitle:(NSString *)customTitle assistant:(WKActionSheetAssistant *)assistant { - NSString *title; - WKElementActionHandlerInternal handler; + NSString *title = @""; + WKElementActionHandlerInternal handler = nil; switch (type) { case _WKElementActionTypeCopy: title = WEB_UI_STRING_KEY("Copy", "Copy (ActionSheet)", "Title for Copy Link or Image action button"); @@ -164,17 +164,9 @@ + (instancetype)_elementActionWithType:(_WKElementActionType)type customTitle:(N [assistant.delegate actionSheetAssistant:assistant shareElementWithURL:actionInfo.URL ?: actionInfo.imageURL rect:actionInfo.boundingRect]; }; break; - case _WKElementActionToggleShowLinkPreviews: { - bool showingLinkPreviews = true; - if (NSNumber *value = [[NSUserDefaults standardUserDefaults] objectForKey:webkitShowLinkPreviewsPreferenceKey]) - showingLinkPreviews = value.boolValue; - - title = showingLinkPreviews ? WEB_UI_STRING("Hide Link Previews", "Title for Hide Link Previews action button") : WEB_UI_STRING("Show Link Previews", "Title for Show Link Previews action button"); - handler = ^(WKActionSheetAssistant *, _WKActivatedElementInfo *) { - [[NSUserDefaults standardUserDefaults] setBool:!showingLinkPreviews forKey:webkitShowLinkPreviewsPreferenceKey]; - }; + case _WKElementActionToggleShowLinkPreviews: + // This action must still exist for compatibility, but doesn't do anything. break; - } default: [NSException raise:NSInvalidArgumentException format:@"There is no standard web element action of type %ld.", (long)type]; return nil; Wouldn't it be better to do this instead so `handler` is not nil (which would cause a crash if `handler` was invoked without a nullptr check--as it is in this class), and future case statements that don't set these variables get a warning/build failure? diff --git a/Source/WebKit/UIProcess/API/Cocoa/_WKElementAction.mm b/Source/WebKit/UIProcess/API/Cocoa/_WKElementAction.mm index 31ec821c9b7..b4b701408f5 100644 --- a/Source/WebKit/UIProcess/API/Cocoa/_WKElementAction.mm +++ b/Source/WebKit/UIProcess/API/Cocoa/_WKElementAction.mm @@ -121,8 +121,8 @@ + (instancetype)_elementActionWithType:(_WKElementActionType)type title:(NSStrin + (instancetype)_elementActionWithType:(_WKElementActionType)type customTitle:(NSString *)customTitle assistant:(WKActionSheetAssistant *)assistant { - NSString *title = @""; - WKElementActionHandlerInternal handler = nil; + NSString *title; + WKElementActionHandlerInternal handler; switch (type) { case _WKElementActionTypeCopy: title = WEB_UI_STRING_KEY("Copy", "Copy (ActionSheet)", "Title for Copy Link or Image action button"); @@ -166,6 +166,8 @@ + (instancetype)_elementActionWithType:(_WKElementActionType)type customTitle:(N break; case _WKElementActionToggleShowLinkPreviews: // This action must still exist for compatibility, but doesn't do anything. + title = @""; + handler = ^(WKActionSheetAssistant *assistant, _WKActivatedElementInfo *actionInfo) { }; break; default: [NSException raise:NSInvalidArgumentException format:@"There is no standard web element action of type %ld.", (long)type];