Bug 119258

Summary: [WebKit2] [Gtk] WebKitResponsePolicyDecision URI response property incorrect
Product: WebKit Reporter: Brian Holt <brian.holt>
Component: WebKitGTKAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: cgarcia, commit-queue, gustavo, mrobinson
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch
none
Updated Patch
none
Updated Patch none

Description Brian Holt 2013-07-30 07:28:26 PDT
g_object_class_install_property(objectClass,
                                    PROP_REQUEST,
                                    g_param_spec_object("response",
                                                      _("URI response"),
                                                      _("The URI response that is associated with this policy decision"),
                                                      WEBKIT_TYPE_URI_REQUEST,
                                                      WEBKIT_PARAM_READABLE));
Comment 1 Brian Holt 2013-07-30 07:44:34 PDT
Created attachment 207736 [details]
Patch
Comment 2 WebKit Commit Bot 2013-07-30 07:47:21 PDT
Thanks for the patch. If this patch contains new public API please make sure it follows the guidelines for new WebKit2 GTK+ API. See http://trac.webkit.org/wiki/WebKitGTK/AddingNewWebKit2API
Comment 3 Brian Holt 2013-07-30 07:50:37 PDT
Comment on attachment 207736 [details]
Patch

Cancelling review: in eclipse I couldn't see the _ that preceded the parentheses.  Apologies.
Comment 4 Brian Holt 2013-07-30 07:52:40 PDT
Created attachment 207737 [details]
Patch
Comment 5 Brian Holt 2013-07-30 08:03:49 PDT
Comment on attachment 207737 [details]
Patch

Thanks for the review! Setting CQ?
Comment 6 Carlos Garcia Campos 2013-07-30 08:04:42 PDT
Comment on attachment 207737 [details]
Patch

What was wrong here? just the coding style?
Comment 7 Carlos Garcia Campos 2013-07-30 08:06:47 PDT
Comment on attachment 207737 [details]
Patch

Ah I see we were using WEBKIT_TYPE_URI_REQUEST instead of WEBKIT_TYPE_URI_RESPONSE, the change was hidden in the conding style "fixes". Please explain what you are fixing in the changelog before landing this.
Comment 8 Brian Holt 2013-07-30 08:13:28 PDT
(In reply to comment #7)
> (From update of attachment 207737 [details])
> Ah I see we were using WEBKIT_TYPE_URI_REQUEST instead of WEBKIT_TYPE_URI_RESPONSE, the change was hidden in the conding style "fixes". Please explain what you are fixing in the changelog before landing this.

Sorry Carlos, I though my comment in the changeLog "Corrected the installed URI response property" was sufficient but I'll expand on that a bit more.  Its definitely not just a style change :-)
Comment 9 Brian Holt 2013-07-30 08:18:24 PDT
Created attachment 207738 [details]
Updated Patch
Comment 10 Carlos Garcia Campos 2013-07-30 08:20:50 PDT
(In reply to comment #8)
> (In reply to comment #7)
> > (From update of attachment 207737 [details] [details])
> > Ah I see we were using WEBKIT_TYPE_URI_REQUEST instead of WEBKIT_TYPE_URI_RESPONSE, the change was hidden in the conding style "fixes". Please explain what you are fixing in the changelog before landing this.
> 
> Sorry Carlos, I though my comment in the changeLog "Corrected the installed URI response property" was sufficient but I'll expand on that a bit more.  Its definitely not just a style change :-)

That says there's something wrong, but not what, it took me a while to find what was wrong. Thanks for updating it.
Comment 11 Carlos Garcia Campos 2013-07-30 08:22:39 PDT
(In reply to comment #9)
> Created an attachment (id=207738) [details]
> Updated Patch

When submitting an updated patch that has already been r+'ed, you can upload the patch with the reviewer line filled, and only ask for commit-queue. Note that I didn't r- the patch, just cq-.
Comment 12 Brian Holt 2013-07-30 08:27:38 PDT
Created attachment 207739 [details]
Updated Patch
Comment 13 Carlos Garcia Campos 2013-07-30 08:35:36 PDT
Comment on attachment 207739 [details]
Updated Patch

Excellent, thanks!
Comment 14 Martin Robinson 2013-07-30 08:47:18 PDT
Thanks for pointing that out Carlos. It took me a moment to find the actual issue. And thanks Brian for fixing it. :)
Comment 15 WebKit Commit Bot 2013-07-30 10:29:42 PDT
Comment on attachment 207739 [details]
Updated Patch

Clearing flags on attachment: 207739

Committed r153479: <http://trac.webkit.org/changeset/153479>
Comment 16 WebKit Commit Bot 2013-07-30 10:29:44 PDT
All reviewed patches have been landed.  Closing bug.