<rdar://problem/13458534>
Created attachment 202461 [details] patch
Attachment 202461 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCore/WebCore.exp.in', u'Source/WebKit2/ChangeLog', u'Source/WebKit2/WebProcess/Plugins/PDF/PDFLayerControllerDetails.h', u'Source/WebKit2/WebProcess/Plugins/PDF/PDFPlugin.h', u'Source/WebKit2/WebProcess/Plugins/PDF/PDFPlugin.mm', u'Source/WebKit2/WebProcess/Plugins/PDF/SimplePDFPlugin.h', u'Source/WebKit2/WebProcess/Plugins/Plugin.h', u'Source/WebKit2/WebProcess/Plugins/PluginView.cpp', u'Source/WebKit2/WebProcess/Plugins/PluginView.h', u'Source/WebKit2/WebProcess/WebPage/WebPage.h', u'Source/WebKit2/WebProcess/WebPage/mac/WKAccessibilityWebPageObject.mm', u'Source/WebKit2/WebProcess/WebPage/mac/WebPageMac.mm']" exit_code: 1 Source/WebKit2/WebProcess/Plugins/PluginView.cpp:788: Declaration has space between type name and * in NSObject *PluginView [whitespace/declaration] [3] Total errors found: 1 in 13 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 202461 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=202461&action=review > Source/WebKit2/WebProcess/Plugins/PDF/PDFPlugin.mm:110 > +- (id)initWithPDFPlugin:(WebKit::PDFPlugin *)plugin; Wrong side *. > Source/WebKit2/WebProcess/Plugins/PDF/PDFPlugin.mm:120 > +- (id)initWithPDFPlugin:(WebKit::PDFPlugin *)plugin Ditto. > Source/WebKit2/WebProcess/Plugins/PDF/PDFPlugin.mm:397 > + m_accessibilityObject = adoptNS([[WKPDFPluginAccessibilityObject alloc] initWithPDFPlugin:this]); This could be above.
(In reply to comment #2) > Attachment 202461 [details] did not pass style-queue: > Source/WebKit2/WebProcess/Plugins/PluginView.cpp:788: Declaration has space between type name and * in NSObject *PluginView [whitespace/declaration] [3] > Total errors found: 1 in 13 files > > > If any of these errors are false positives, please file a bug against check-webkit-style. You're a false positive :\
Comment on attachment 202461 [details] patch Attachment 202461 [details] did not pass efl-wk2-ews (efl-wk2): Output: http://webkit-queues.appspot.com/results/292228
Comment on attachment 202461 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=202461&action=review overall looks good. just a few minor nits. > Source/WebKit2/WebProcess/Plugins/PDF/PDFLayerControllerDetails.h:121 > +- (CPReadingModel*)readingModel; space before the * > Source/WebKit2/WebProcess/Plugins/PDF/PDFPlugin.mm:217 > + return [NSArray array]; we should probably expose the NSAccessibilityShowMenuAction since I believe you can right click on the PDF > Source/WebKit2/WebProcess/Plugins/PDF/SimplePDFPlugin.h:148 > + virtual NSObject *accessibilityObject() const OVERRIDE { return 0; } is this needed?
Comment on attachment 202461 [details] patch Attachment 202461 [details] did not pass qt-wk2-ews (qt-wk2): Output: http://webkit-queues.appspot.com/results/524273
(In reply to comment #6) > (From update of attachment 202461 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=202461&action=review > > overall looks good. just a few minor nits. > > > Source/WebKit2/WebProcess/Plugins/PDF/PDFLayerControllerDetails.h:121 > > +- (CPReadingModel*)readingModel; > > space before the * Yep. > > Source/WebKit2/WebProcess/Plugins/PDF/PDFPlugin.mm:217 > > + return [NSArray array]; > > we should probably expose the NSAccessibilityShowMenuAction > since I believe you can right click on the PDF Good idea! > > Source/WebKit2/WebProcess/Plugins/PDF/SimplePDFPlugin.h:148 > > + virtual NSObject *accessibilityObject() const OVERRIDE { return 0; } > > is this needed? Perhaps not!
Created attachment 202841 [details] patch
Created attachment 202842 [details] patch
Attachment 202842 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCore/WebCore.exp.in', u'Source/WebKit2/ChangeLog', u'Source/WebKit2/WebProcess/Plugins/PDF/PDFLayerControllerDetails.h', u'Source/WebKit2/WebProcess/Plugins/PDF/PDFPlugin.h', u'Source/WebKit2/WebProcess/Plugins/PDF/PDFPlugin.mm', u'Source/WebKit2/WebProcess/Plugins/PDF/SimplePDFPlugin.h', u'Source/WebKit2/WebProcess/Plugins/Plugin.h', u'Source/WebKit2/WebProcess/Plugins/PluginView.cpp', u'Source/WebKit2/WebProcess/Plugins/PluginView.h', u'Source/WebKit2/WebProcess/WebPage/WebPage.h', u'Source/WebKit2/WebProcess/WebPage/mac/WKAccessibilityWebPageObject.mm', u'Source/WebKit2/WebProcess/WebPage/mac/WebPageMac.mm']" exit_code: 1 Source/WebKit2/WebProcess/Plugins/PDF/PDFPlugin.h:96: The parameter name "point" adds no information, so it should be removed. [readability/parameter_name] [5] Source/WebKit2/WebProcess/Plugins/PluginView.cpp:523: Declaration has space between type name and * in NSObject *PluginView [whitespace/declaration] [3] Total errors found: 2 in 13 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 202843 [details] patch
Attachment 202843 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCore/WebCore.exp.in', u'Source/WebKit2/ChangeLog', u'Source/WebKit2/WebProcess/Plugins/PDF/PDFLayerControllerDetails.h', u'Source/WebKit2/WebProcess/Plugins/PDF/PDFPlugin.h', u'Source/WebKit2/WebProcess/Plugins/PDF/PDFPlugin.mm', u'Source/WebKit2/WebProcess/Plugins/PDF/SimplePDFPlugin.h', u'Source/WebKit2/WebProcess/Plugins/Plugin.h', u'Source/WebKit2/WebProcess/Plugins/PluginView.cpp', u'Source/WebKit2/WebProcess/Plugins/PluginView.h', u'Source/WebKit2/WebProcess/WebPage/WebPage.h', u'Source/WebKit2/WebProcess/WebPage/mac/WKAccessibilityWebPageObject.mm', u'Source/WebKit2/WebProcess/WebPage/mac/WebPageMac.mm']" exit_code: 1 Source/WebKit2/WebProcess/Plugins/PluginView.cpp:523: Declaration has space between type name and * in NSObject *PluginView [whitespace/declaration] [3] Total errors found: 1 in 13 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 202843 [details] patch Can we test this? r=me
(In reply to comment #14) > (From update of attachment 202843 [details]) > Can we test this? > > r=me Not yet! Thanks!
http://trac.webkit.org/changeset/150771