Bug 92878 – [GTK] Add WebKitNetworkResponse::suggested-filename property

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKitGTK

[Claudio Saavedra]

Webcore has API the suggested filename for a response, add a property and getter for it in WebKitNetworkResponse. Patch for WK1 follows.

[Claudio Saavedra]

Created attachment 155825 [details]
2012-08-01  Claudio Saavedra  <[e-mail]>

[GTK] Add WebKitNetworkResponse::suggested-filename property
        Need the bug URL (OOPS!).

        Reviewed by NOBODY (OOPS!).

        Webcore has API the suggested filename for a response, add
        a property and getter for it in WebKitNetworkResponse.

        * docs/webkitgtk-sections.txt: Add the symbols
        * webkit/webkitnetworkresponse.cpp:
        (_WebKitNetworkResponsePrivate): Add suggested_filename.
        (webkit_network_response_finalize): Free it on finalize
        (webkit_network_response_get_property): Add the getter call.
        (webkit_network_response_class_init): Install the property.
        (webkit_network_response_get_suggested_filename): New getter.
        * webkit/webkitnetworkresponse.h: Add the declaration to header
        file.

[Martin Robinson]

Comment on attachment 155825 [details]
2012-08-01  Claudio Saavedra  <[e-mail]>

View in context: https://bugs.webkit.org/attachment.cgi?id=155825&action=review

Looks good to me, apart from some style nits.

> Source/WebKit/gtk/webkit/webkitnetworkresponse.cpp:44
> +using namespace WebKit;
> +

This should really be just below the header list.

> Source/WebKit/gtk/webkit/webkitnetworkresponse.cpp:174
> +                                                        (GParamFlags)(WEBKIT_PARAM_READABLE)));

Should use a C++ cast here.

> Source/WebKit/gtk/webkit/webkitnetworkresponse.cpp:287
> +    g_return_val_if_fail(WEBKIT_IS_NETWORK_RESPONSE(response), NULL);

Instead of NULL use 0 in WebKit, unless it's the end of an argument list.

[Carlos Garcia Campos]

Comment on attachment 155825 [details]
2012-08-01  Claudio Saavedra  <[e-mail]>

View in context: https://bugs.webkit.org/attachment.cgi?id=155825&action=review

API looks good to me

>> Source/WebKit/gtk/webkit/webkitnetworkresponse.cpp:174
>> +                                                        (GParamFlags)(WEBKIT_PARAM_READABLE)));
> 
> Should use a C++ cast here.

We don't need the cast, I think , since it's a single flag.

[Carlos Garcia Campos]

Comment on attachment 155825 [details]
2012-08-01  Claudio Saavedra  <[e-mail]>

View in context: https://bugs.webkit.org/attachment.cgi?id=155825&action=review

> Source/WebKit/gtk/webkit/webkitnetworkresponse.cpp:280
> + * Obtains the suggested filename for the given network response.

Maybe we could explain this is extracted from the Content-Disposition HTTP header, and that it might not be present

> Source/WebKit/gtk/webkit/webkitnetworkresponse.cpp:282
> + * Returns: (transfer none): the suggested filename

We should mention this can return NULL

[Claudio Saavedra]

Created attachment 155831 [details]
2012-08-01  Claudio Saavedra  <[e-mail]>

[GTK] Add WebKitNetworkResponse::suggested-filename property
        Need the bug URL (OOPS!).

        Reviewed by NOBODY (OOPS!).

        Webcore has API the suggested filename for a response, add
        a property and getter for it in WebKitNetworkResponse.

        * docs/webkitgtk-sections.txt: Add the symbols
        * webkit/webkitnetworkresponse.cpp:
        (_WebKitNetworkResponsePrivate): Add suggested_filename.
        (webkit_network_response_finalize): Free it on finalize
        (webkit_network_response_get_property): Add the getter call.
        (webkit_network_response_class_init): Install the property.
        (webkit_network_response_get_suggested_filename): New getter.
        * webkit/webkitnetworkresponse.h: Add the declaration to header
        file.

[Carlos Garcia Campos]

Comment on attachment 155831 [details]
2012-08-01  Claudio Saavedra  <[e-mail]>

View in context: https://bugs.webkit.org/attachment.cgi?id=155831&action=review

> Source/WebKit/gtk/webkit/webkitnetworkresponse.cpp:173
> +                                                        NULL,

0 instead of NULL here too :-P

[Claudio Saavedra]

Created attachment 155837 [details]
2012-08-01  Claudio Saavedra  <[e-mail]>

[GTK] Add WebKitNetworkResponse::suggested-filename property
        Need the bug URL (OOPS!).

        Reviewed by NOBODY (OOPS!).

        Webcore has API the suggested filename for a response, add
        a property and getter for it in WebKitNetworkResponse.

        * docs/webkitgtk-sections.txt: Add the symbols
        * webkit/webkitnetworkresponse.cpp:
        (_WebKitNetworkResponsePrivate): Add suggested_filename.
        (webkit_network_response_finalize): Free it on finalize
        (webkit_network_response_get_property): Add the getter call.
        (webkit_network_response_class_init): Install the property.
        (webkit_network_response_get_suggested_filename): New getter.
        * webkit/webkitnetworkresponse.h: Add the declaration to header
        file.

[Carlos Garcia Campos]

Comment on attachment 155837 [details]
2012-08-01  Claudio Saavedra  <[e-mail]>

View in context: https://bugs.webkit.org/attachment.cgi?id=155837&action=review

> Source/WebKit/gtk/webkit/webkitnetworkresponse.cpp:49
> +    gchar* suggested_filename;

Sorry I didn't notice this before, and I wonder why the style checker haven't complained, but this should be suggestedFilename instead of suggested_filename.

[Claudio Saavedra]

Created attachment 155991 [details]
2012-08-01  Claudio Saavedra  <[e-mail]>

[GTK] Add WebKitNetworkResponse::suggested-filename property
        Need the bug URL (OOPS!).

        Reviewed by NOBODY (OOPS!).

        Webcore has API the suggested filename for a response, add
        a property and getter for it in WebKitNetworkResponse.

        * docs/webkitgtk-sections.txt: Add the symbols
        * webkit/webkitnetworkresponse.cpp:
        (_WebKitNetworkResponsePrivate): Add suggested_filename.
        (webkit_network_response_finalize): Free it on finalize
        (webkit_network_response_get_property): Add the getter call.
        (webkit_network_response_class_init): Install the property.
        (webkit_network_response_get_suggested_filename): New getter.
        * webkit/webkitnetworkresponse.h: Add the declaration to header
        file.

[Claudio Saavedra]

Comment on attachment 155991 [details]
2012-08-01  Claudio Saavedra  <[e-mail]>

Fixed the casing of the private variable for the suggested filename.

[Xan Lopez]

Comment on attachment 155991 [details]
2012-08-01  Claudio Saavedra  <[e-mail]>

This looks good to me too!

[WebKit Review Bot]

Comment on attachment 155991 [details]
2012-08-01  Claudio Saavedra  <[e-mail]>

Clearing flags on attachment: 155991

Committed r124427: <http://trac.webkit.org/changeset/124427>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.