Fix "Open with Preview" menu item in PDF context menus on Big Sur
Created attachment 416440 [details] Patch
<rdar://problem/72406073>
using Arguments = std::tuple<const WebKit::PDFContextMenu&, const WebKit::PDFPluginIdentifier&>; ~~~~~~~~^ /Volumes/Data/worker/macOS-Mojave-Release-Build-EWS/build/WebKitBuild/Release/DerivedSources/WebKit2/WebPageProxyMessages.h:5784:81: error: no type named 'PDFPluginIdentifier' in namespace 'WebKit' ShowPDFContextMenu(const WebKit::PDFContextMenu& contextMenu, const WebKit::PDFPluginIdentifier& identifier) ~~~~~~~~^ /Volumes/Data/worker/macOS-Mojave-Release-Build-EWS/build/WebKitBuild/Release/DerivedSources/WebKit2/WebPageProxyMessages.h:5789:11: error: unknown type name 'Arguments' const Arguments& arguments() const ^ /Volumes/Data/worker/macOS-Mojave-Release-Build-EWS/build/WebKitBuild/Release/DerivedSources/WebKit2/WebPageProxyMessages.h:5795:5: error: unknown type name 'Arguments'
Comment on attachment 416440 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=416440&action=review > Source/WebKit/WebProcess/Plugins/PDF/PDFPlugin.mm:2333 > + Optional<int> openInPreviewIndex; > Vector<PDFContextMenuItem> items; > auto itemCount = [nsMenu numberOfItems]; > for (int i = 0; i < itemCount; i++) { > auto item = [nsMenu itemAtIndex:i]; > if ([item submenu]) > continue; > + if ([NSStringFromSelector(item.action) isEqualToString:@"openWithPreview"]) > + openInPreviewIndex = i; > PDFContextMenuItem menuItem { String([item title]), !![item isEnabled], !![item isSeparatorItem], static_cast<int>([item state]), !![item action], i }; > items.append(WTFMove(menuItem)); > } > - PDFContextMenu contextMenu { point, WTFMove(items) }; > + PDFContextMenu contextMenu { point, WTFMove(items), WTFMove(openInPreviewIndex) }; Why do we compute openInPreviewIndex in the WebContent process? Are we not able to compute it in the UI Process? (I think we might be able to compute it in the UI Process inside WebPageProxy::showPDFContextMenu.)
We are unable to compute it in the UI process because the only attributes it has to distinguish it are its title, which is a localized string that we can't do unlocalized string comparison with, and its selector, which only exists in the web process.
Created attachment 416442 [details] Patch
Created attachment 416444 [details] Patch
Comment on attachment 416444 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=416444&action=review r=me Can an API test use a fake mouse event to test this in at least a simple case? > Source/WebKit/WebProcess/Plugins/PDF/PDFPlugin.mm:2327 > + if ([NSStringFromSelector(item.action) isEqualToString:@"openWithPreview"]) > + openInPreviewIndex = i; I think this means that a rename inside the PDFLayerController project would reintroduce this bug. Could you file a request for explicit support for getting this item index?
Comment on attachment 416444 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=416444&action=review >> Source/WebKit/WebProcess/Plugins/PDF/PDFPlugin.mm:2327 >> + openInPreviewIndex = i; > > I think this means that a rename inside the PDFLayerController project would reintroduce this bug. > > Could you file a request for explicit support for getting this item index? This is one of the many, many bugs related to PDFs that we don't have a good way to test.
Committed r270946: <https://trac.webkit.org/changeset/270946> All reviewed patches have been landed. Closing bug and clearing flags on attachment 416444 [details].