[chromium] Add support to allow printing just a plugin in a frame
Created attachment 78358 [details] Patch
btw the corresponding Chromium bug is http://crbug.com/50285
Created attachment 78606 [details] Patch
Attachment 78606 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'WebKit/chromium/ChangeLog', u'WebKit/chrom..." exit_code: 1 WebKit/chromium/public/WebFrame.h:409: The parameter name "node" adds no information, so it should be removed. [readability/parameter_name] [5] Total errors found: 1 in 5 files If any of these errors are false positives, please file a bug against check-webkit-style.
this is now ready for review, while I figure out the objc gibberish on the browse side :)
Attachment 78606 [details] did not build on chromium: Build output: http://queues.webkit.org/results/7378150
(In reply to comment #6) > Attachment 78606 [details] did not build on chromium: > Build output: http://queues.webkit.org/results/7378150 doh, forgot about the n-sided patches. will check in a chrome change now that makes this compile and update the deps in this.
Created attachment 78621 [details] Patch
Attachment 78621 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'WebKit/chromium/ChangeLog', u'WebKit/chrom..." exit_code: 1 WebKit/chromium/public/WebFrame.h:412: The parameter name "node" adds no information, so it should be removed. [readability/parameter_name] [5] Total errors found: 1 in 6 files If any of these errors are false positives, please file a bug against check-webkit-style.
Attachment 78621 [details] did not build on chromium: Build output: http://queues.webkit.org/results/7402137
Created attachment 78628 [details] Patch
Attachment 78628 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'WebKit/chromium/ChangeLog', u'WebKit/chrom..." exit_code: 1 WebKit/chromium/public/WebFrame.h:412: The parameter name "node" adds no information, so it should be removed. [readability/parameter_name] [5] WebKit/chromium/src/WebFrameImpl.h:154: The parameter name "node" adds no information, so it should be removed. [readability/parameter_name] [5] Total errors found: 2 in 7 files If any of these errors are false positives, please file a bug against check-webkit-style.
(In reply to comment #12) > Attachment 78628 [details] did not pass style-queue: > > Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'WebKit/chromium/ChangeLog', u'WebKit/chrom..." exit_code: 1 > WebKit/chromium/public/WebFrame.h:412: The parameter name "node" adds no information, so it should be removed. [readability/parameter_name] [5] > WebKit/chromium/src/WebFrameImpl.h:154: The parameter name "node" adds no information, so it should be removed. [readability/parameter_name] [5] > Total errors found: 2 in 7 files > > > If any of these errors are false positives, please file a bug against check-webkit-style. It looks like the style bot is trying to tell you something. (fwiw, it's correct).
(In reply to comment #13) > (In reply to comment #12) > > Attachment 78628 [details] [details] did not pass style-queue: > > > > Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'WebKit/chromium/ChangeLog', u'WebKit/chrom..." exit_code: 1 > > WebKit/chromium/public/WebFrame.h:412: The parameter name "node" adds no information, so it should be removed. [readability/parameter_name] [5] > > WebKit/chromium/src/WebFrameImpl.h:154: The parameter name "node" adds no information, so it should be removed. [readability/parameter_name] [5] > > Total errors found: 2 in 7 files > > > > > > If any of these errors are false positives, please file a bug against check-webkit-style. > > It looks like the style bot is trying to tell you something. (fwiw, it's correct). in this case, it looked to me like it would make the code more awkward. i.e. the example given in the style guide only has one parameter and no default value. but in this code: virtual int printBegin(const WebSize& pageSize, WebNode* = 0, int printerDPI = 72, bool* useBrowserOverlays = 0) = 0; it looks a little strange to me having a function declaration with 3 named variables and one that's not named, especially if it has a default value. it makes referencing the variable in a comment inconsistent with how the others are referenced, since |node| doesn't make sense anymore. but, i don't care enough, if the script wants inconsistencies within one function declaration, so be it..
Created attachment 78661 [details] Patch
(In reply to comment #14) > (In reply to comment #13) > > (In reply to comment #12) > > > Attachment 78628 [details] [details] [details] did not pass style-queue: > > > > > > Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'WebKit/chromium/ChangeLog', u'WebKit/chrom..." exit_code: 1 > > > WebKit/chromium/public/WebFrame.h:412: The parameter name "node" adds no information, so it should be removed. [readability/parameter_name] [5] > > > WebKit/chromium/src/WebFrameImpl.h:154: The parameter name "node" adds no information, so it should be removed. [readability/parameter_name] [5] > > > Total errors found: 2 in 7 files > > > > > > > > > If any of these errors are false positives, please file a bug against check-webkit-style. > > > > It looks like the style bot is trying to tell you something. (fwiw, it's correct). > > in this case, it looked to me like it would make the code more awkward. i.e. the example given in the style guide only has one parameter and no default value. but in this code: > > virtual int printBegin(const WebSize& pageSize, > WebNode* = 0, > int printerDPI = 72, > bool* useBrowserOverlays = 0) = 0; > > it looks a little strange to me having a function declaration with 3 named variables and one that's not named, especially if it has a default value. It sounds like you are used to a different style other than WebKit so it looks strange. Ok. > it makes referencing the variable in a comment inconsistent with how the others are referenced, since |node| doesn't make sense anymore. Feel free to say WebNode*. > but, i don't care enough, if the script wants inconsistencies within one function declaration, so be it.. WebKit style is to omit parameter names that don't add information. The script is correctly reminding you of this. This isn't about some script wanting inconsistencies.
Comment on attachment 78661 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=78661&action=review > WebKit/chromium/public/WebFrame.h:411 > + virtual int printBegin(const WebSize& pageSize, if it makes your merging easier, it is also OK to put the WebNode parameter last, and i might give it a self-documenting name. it would also be a bit more conventional to pass the WebNode like this: const WebNode& constrainToNode = WebNode(), Note: WebNode has an isNull method you can check. > WebKit/chromium/src/WebFrameImpl.cpp:1285 nit: it is a bit more conventional to name webcore objects using the prefix "core", so "coreNode" instead of "webcoreNode" nit: you can also just do this: Node* coreNode = node->unwrap<WebCore::Node>(); if you pass as const WebNode& node, then you would do this: const Node* coreNode = node.constUnwrap<Node>();
Created attachment 78752 [details] Patch
(In reply to comment #17) > (From update of attachment 78661 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=78661&action=review > > > WebKit/chromium/public/WebFrame.h:411 > > + virtual int printBegin(const WebSize& pageSize, > > if it makes your merging easier, it is also OK to put the WebNode parameter last, > and i might give it a self-documenting name. > > it would also be a bit more conventional to pass the WebNode like this: > > const WebNode& constrainToNode = WebNode(), > > Note: WebNode has an isNull method you can check. > > > WebKit/chromium/src/WebFrameImpl.cpp:1285 > > > nit: it is a bit more conventional to name webcore objects using the prefix "core", so "coreNode" instead of "webcoreNode" > > nit: you can also just do this: > > Node* coreNode = node->unwrap<WebCore::Node>(); > > if you pass as const WebNode& node, then you would do this: > > const Node* coreNode = node.constUnwrap<Node>(); Done and uploaded
Committed r75730: <http://trac.webkit.org/changeset/75730>