WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
219986
Fix "Open with Preview" menu item in PDF context menus on Big Sur
https://bugs.webkit.org/show_bug.cgi?id=219986
Summary
Fix "Open with Preview" menu item in PDF context menus on Big Sur
Alex Christensen
Reported
2020-12-17 11:10:29 PST
Fix "Open with Preview" menu item in PDF context menus on Big Sur
Attachments
Patch
(8.36 KB, patch)
2020-12-17 11:14 PST
,
Alex Christensen
ews-feeder
: commit-queue-
Details
Formatted Diff
Diff
Patch
(8.87 KB, patch)
2020-12-17 11:31 PST
,
Alex Christensen
ews-feeder
: commit-queue-
Details
Formatted Diff
Diff
Patch
(9.82 KB, patch)
2020-12-17 11:39 PST
,
Alex Christensen
no flags
Details
Formatted Diff
Diff
Show Obsolete
(2)
View All
Add attachment
proposed patch, testcase, etc.
Alex Christensen
Comment 1
2020-12-17 11:14:53 PST
Created
attachment 416440
[details]
Patch
Alex Christensen
Comment 2
2020-12-17 11:14:56 PST
<
rdar://problem/72406073
>
Geoffrey Garen
Comment 3
2020-12-17 11:24:57 PST
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'
Geoffrey Garen
Comment 4
2020-12-17 11:27:18 PST
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.)
Alex Christensen
Comment 5
2020-12-17 11:29:26 PST
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.
Alex Christensen
Comment 6
2020-12-17 11:31:21 PST
Created
attachment 416442
[details]
Patch
Alex Christensen
Comment 7
2020-12-17 11:39:41 PST
Created
attachment 416444
[details]
Patch
Geoffrey Garen
Comment 8
2020-12-17 12:34:45 PST
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?
Alex Christensen
Comment 9
2020-12-17 14:12:25 PST
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.
EWS
Comment 10
2020-12-17 14:21:34 PST
Committed
r270946
: <
https://trac.webkit.org/changeset/270946
> All reviewed patches have been landed. Closing bug and clearing flags on
attachment 416444
[details]
.
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug