Summary: | Need a WK1 Mac API to filter which subframes go into WebArchives as they are created | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Brady Eidson <beidson> | ||||||
Component: | WebKit API | Assignee: | Brady Eidson <beidson> | ||||||
Status: | ASSIGNED --- | ||||||||
Severity: | Normal | CC: | commit-queue | ||||||
Priority: | P2 | Keywords: | InRadar | ||||||
Version: | 528+ (Nightly build) | ||||||||
Hardware: | Mac | ||||||||
OS: | All | ||||||||
Attachments: |
|
Description
Brady Eidson
2012-02-23 16:49:19 PST
Created attachment 128606 [details]
Patch v1
Comment on attachment 128606 [details] Patch v1 View in context: https://bugs.webkit.org/attachment.cgi?id=128606&action=review > Source/WebKit/mac/DOM/WebDOMOperationsPrivate.h:53 > +@interface DOMNode (WebDOMNodeOperationsPrivate) > +- (WebArchive *)webArchiveWithWebArchiveCreationDelegate:(id)webArchiveCreationDelegate; > +@end This should take a block, not an arbitrary object. Created attachment 128631 [details]
Patch v2 - Use a block
Attachment 128631 [details] did not pass style-queue:
Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebKit/mac/ChangeLog', u'Source/Web..." exit_code: 1
Source/WebKit/mac/DOM/WebDOMOperationsPrivate.h:53: Extra space before ( in function call [whitespace/parens] [4]
Total errors found: 1 in 3 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 128631 [details] Patch v2 - Use a block View in context: https://bugs.webkit.org/attachment.cgi?id=128631&action=review Feel free to add an API test as well. > Source/WebKit/mac/DOM/WebDOMOperations.mm:89 > +class WebFrameFilter : public WebCore::FrameFilter { > +public: > + WebFrameFilter(ShouldIncludeSubframeInWebArchiveBlock filterBlock); > + > +private: > + virtual bool shouldIncludeSubframe(Frame*) const OVERRIDE; > + > + ShouldIncludeSubframeInWebArchiveBlock m_filterBlock; > +}; Though perhaps not strictly necessary due to the one place it is used, it seems safer to Block_copy/Block_release this in the c++ class. Comment on attachment 128631 [details] Patch v2 - Use a block View in context: https://bugs.webkit.org/attachment.cgi?id=128631&action=review > Source/WebKit/mac/DOM/WebDOMOperationsPrivate.h:54 > +- (WebArchive *)webArchiveFilteringSubframes:(ShouldIncludeSubframeInWebArchiveBlock)shouldIncludeSubframeInWebArchive; Maybe webArchiveWithSubframeFilter: would be a better name? And the block typedef could be WebArchiveSubframeFilter. (I'm not sure the block typedef needs to include the word "block".) Comment on attachment 128631 [details] Patch v2 - Use a block View in context: https://bugs.webkit.org/attachment.cgi?id=128631&action=review >> Source/WebKit/mac/DOM/WebDOMOperationsPrivate.h:54 >> +- (WebArchive *)webArchiveFilteringSubframes:(ShouldIncludeSubframeInWebArchiveBlock)shouldIncludeSubframeInWebArchive; > > Maybe webArchiveWithSubframeFilter: would be a better name? And the block typedef could be WebArchiveSubframeFilter. (I'm not sure the block typedef needs to include the word "block".) After all, this block is just being used to perform a functional-style filter operation. |