[webkit-reviews] review denied: [Bug 56849] REGRESSION: WK2: AX: PDF in Safari no longer accessible. : [Attachment 86484] patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Tue Mar 22 18:09:53 PDT 2011


Darin Adler <darin at apple.com> has denied chris fleizach <cfleizach at apple.com>'s
request for review:
Bug 56849: REGRESSION: WK2: AX: PDF in Safari no longer accessible.
https://bugs.webkit.org/show_bug.cgi?id=56849

Attachment 86484: patch
https://bugs.webkit.org/attachment.cgi?id=86484&action=review

------- Additional Comments from Darin Adler <darin at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=86484&action=review

review- because of the bad cast; otherwise looks good

> Source/WebKit2/UIProcess/API/mac/WKView.mm:1611
> +	   return [NSArray arrayWithObjects:child, nil];

This will return an empty array instead of nil in cases where the old code
returned nil. How about calling [NSArray arrayWithObject:] explicitly in both
cases above instead of using a local variable?

> Source/WebKit2/UIProcess/API/mac/PDFViewController.h:55
> +    WKView* pdfView() const { return
static_cast<WKView*>(m_wkPDFView.get()); }

This is a bad cast, and incorrect. This function should be not be inline. I
should be in the .mm file where the compiler can see the WKPDFView class. And
the return type should be NSView *.


More information about the webkit-reviews mailing list