[Webkit-unassigned] [Bug 4151] USPS.com Label Printing Malfunction

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=4151


Anders Carlsson <andersca at apple.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #39827|review?                     |review-
               Flag|                            |




--- Comment #15 from Anders Carlsson <andersca at apple.com>  2009-09-21 10:35:22 PDT ---
(From update of attachment 39827)
> +#import <JavaScriptCore/JSBase.h>
> +
> + at class WebDataSource;
> +
> +#ifdef __cplusplus
> +extern "C" {
> +#endif
> +JSObjectRef makeJSPDFDoc(JSContextRef, WebDataSource *);
> +#ifdef __cplusplus
> +}
> +#endif

Why does this need to be extern "C"?

> +static void jsPDFDocInitialize(JSContextRef ctx, JSObjectRef object)
> +{
> +    WebDataSource *dataSource = (WebDataSource *)JSObjectGetPrivate(object);
> +    [dataSource retain];
> +}

This should use CFRetain or it'll break under GC

> +
> +static void jsPDFDocFinalize(JSObjectRef object)
> +{
> +    WebDataSource *dataSource = (WebDataSource *)JSObjectGetPrivate(object);
> +    [dataSource release];
> +}

Same here, use CFRelease.

> +
> +static JSValueRef jsPDFDocPrint(JSContextRef ctx, JSObjectRef function, JSObjectRef thisObject, size_t argumentCount, const JSValueRef arguments[], JSValueRef* exception)
> +{
> +    WebDataSource *dataSource = (WebDataSource *)JSObjectGetPrivate(thisObject);
> +
> +    WebView *webView = [[dataSource webFrame] webView];
> +    CallUIDelegate(webView, @selector(webView:printFrameView:), [[dataSource webFrame] frameView]);

Is there no internal method for printing a frame view?

> +
> +    return JSValueMakeNull(ctx);

"void" javascript functions usually return undefined. Are you sure this should
return null?

> +
> +#ifdef __cplusplus
> +extern "C" {
> +#endif
> +void addWebPDFDocumentExtras(Class);
> +#ifdef __cplusplus
> +}
> +#endif

Again, why does this need to be extern C?

> +
> +void addWebPDFDocumentExtras(Class pdfDocumentClass)
> +{
> +#ifndef BUILDING_ON_TIGER
> +    class_addMethod(pdfDocumentClass, @selector(_web_allScripts), (IMP)web_PDFDocumentAllScripts, "@@:");
> +#else
> +    struct objc_method_list methodList = { 0, 1, { @selector(_web_allScripts), (IMP)web_PDFDocumentAllScripts, "@@:" } };
> +    class_addMethods(pdfDocumentClass, &methodList);
> +#endif

Why do you need to add this to the class? Can't you just use a category method?

> ++ (void)initialize
> +{
> +    Class pdfDocumentClass = [self PDFDocumentClass];
> +    if (pdfDocumentClass)
> +        addWebPDFDocumentExtras(pdfDocumentClass);

+initialize should check if self == [WebPDFRepresentation class] to guard
against adding the same extras more than once.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.