[Webkit-unassigned] [Bug 87283] [GTK] run-file-chooser signal for WebKit1

[bugzilla-daemon at webkit.org]

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





--- Comment #11 from Mario Sanchez Prada <msanchez at igalia.com>  2012-05-29 08:07:51 PST ---
The patch looks great. So great it looks that I can't do much more on my side other than making some final comments (see below). From now on an official reviewer should take the lead and give the final approval (or not).

In the meanwhile, I'd recommend you to prepare the ChangeLog entry. It's the only thing missing I can spot (and sorry not to have spotted it before).

(In reply to comment #10)
> Created an attachment (id=144555)
 --> (https://bugs.webkit.org/attachment.cgi?id=144555&action=review) [details]
> updated patch for review
> 
> Thanks! Here's a new patch addressing those comments.

Great. I think now you only need to run the prepareChangeLog script and provide a properly formatted entry in Source/WebKit/gtk/ChangeLog.

You can do it manually too, but if so make sure you format it properly, avoid tabs (only spaces) and that you include a line like the following one:

  Reviewed by NOBODY (OOPS!).

(This line will get changed with the name of the reviewe on it once it gets r+ and while pushing it to the repository)

> If there is not much focus on making WebKit1 and WebKit2 gobject APIs 
> similar, I agree with removing the cancel method (done in the patch).

The idea is to make them similar, but not necessarily identical. In this case I think it makes sense to leave that function out of the game.

> The one remaining issue I have here is that it complains during build:
> 
> /home/dsd/projects/WebKit-r118336/Source/WebKit/gtk/webkit/webkitfilechooserrequest.cpp:56:
> warning: Section WebKitFileChooserRequest is not defined in the webkitgtk-section.txt file.
> 
> Does it mean webkitgtk-sections.txt (with an 's')? I do have an entry there, and I can't
> see what I'm doing differently compared to other classes. Either way it does not seem to
> affect the functionality.

I've seen this problem going away after forcing a full clean rebuild. Not sure if it's the case here, though.

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