Summary: | [Chromium] Enable Chromium mac to display a context menu. | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Chris Guillory <ctguil> | ||||||
Component: | Accessibility | Assignee: | Nobody <webkit-unassigned> | ||||||
Status: | RESOLVED WONTFIX | ||||||||
Severity: | Normal | CC: | cfleizach, darin, fishd, schenney, webkit.review.bot | ||||||
Priority: | P2 | ||||||||
Version: | 528+ (Nightly build) | ||||||||
Hardware: | All | ||||||||
OS: | All | ||||||||
Attachments: |
|
Description
Chris Guillory
2010-12-07 17:23:48 PST
Created attachment 75860 [details]
Patch
Chromium WebKit API change. Created attachment 76728 [details]
Proposed Patch - Add WebView::showContextMenu(const WebAccessibilityObject&)
Attachment 76728 [details] did not pass style-queue: Failed to run "['WebKitTools/Scripts/check-webkit-style', '--diff-files', u'WebKit/chromium/ChangeLog', u'WebKit/chromium/public/WebAccessibilityObject.h', u'WebKit/chromium/public/WebView.h', u'WebKit/chromium/src/WebAccessibilityObject.cpp', u'WebKit/chromium/src/WebViewImpl.cpp', u'WebKit/chromium/src/WebViewImpl.h']" exit_code: 1 WebKit/chromium/src/WebViewImpl.cpp:1923: Local variables should never be PassRefPtr (see http://webkit.org/coding/RefPtr.html). [readability/pass_ptr] [5] Total errors found: 1 in 6 files If any of these errors are false positives, please file a bug against check-webkit-style. Comment on attachment 76728 [details]
Proposed Patch - Add WebView::showContextMenu(const WebAccessibilityObject&)
how come this can't call into the method in AccessibilityObject to show the context menu?
Comment on attachment 76728 [details] Proposed Patch - Add WebView::showContextMenu(const WebAccessibilityObject&) View in context: https://bugs.webkit.org/attachment.cgi?id=76728&action=review > WebKit/chromium/public/WebView.h:278 > + nit: avoid adding lines with just spaces > WebKit/chromium/public/WebView.h:282 > + virtual void showContextMenu(const WebAccessibilityObject& obj) = 0; nit: the API header should not refer to implementation details, especially not things in WebCore. WebCore could change and no one would know to update this comment. it also doesn't help someone use the API correctly. I think it might be more useful API-wise to generalize this method. You could generalize it like this: virtual void showContextMenu(const WebNode&, const WebPoint& position); But, perhaps it would be better to create a sibling to WebNode::simulateClick: WEBKIT_API void simulateContextMenu(); Perhaps you should implement it using Node::dispatchSimulatedMouseEvent? I'm not certain about that though. Comment on attachment 76728 [details]
Proposed Patch - Add WebView::showContextMenu(const WebAccessibilityObject&)
r- based on comments from Darin and stylebot issue.
I have to assume this has been dealt with by now. |