Bug 15255 – GTK+ WebKit should allow the application to execute Javascript on it.

• Component: WebKit API
• Status: RESOLVED FIXED

[Sean Egan]

I'm implementing AdiumX "chat styles" (http://adiumxtras.com) in Pidgin (http://pidgin.im). These chat styles require the application to call a javascript function defined by the style.

This patch allows the GTK+ bindings to call arbitrary javascript on the WebkitPage.

An alternative is to have webkit_gtk_page_open call the change_location method, which special cases javascript: uri's and use that method with javascript: uri's, but this seemed like the better solution to me.

Just to show off a bit, some screenshots of what this allows: http://pidgin.im/~seanegan/webkit3.png http://pidgin.im/~seanegan/webkit4.png http://pidgin.im/~seanegan/webkit5.png http://pidgin.im/~seanegan/webkit6.gif

[Sean Egan]

Created attachment 16345 [details]
allow application to call javascript on a WebkitPage

[Sean Egan]

Created attachment 16347 [details]
Call Javascript from gtkwebkitpage

[Sean Egan]

Whoops! My first patch still had some test code in it. Removed.

[Adam Roben (:aroben)]

Comment on attachment 16347 [details]
Call Javascript from gtkwebkitpage

As per <http://webkit.org/coding/coding-style.html>, please put asterisks next to the type name, not the variable name.

+    if (FrameLoader *loader = frameData->frame->loader())
+        loader->executeScript(script, true);
+	
+}

Looks like you've got an extra line of whitespace after the if statement.

I also think it would make more sense for this API to be on WebKitGtkFrame, rather than having it implicitly operate on the main frame.

r- for now so that the above changes can be made, but I definitely agree this is a useful API to have!

[Sean Egan]

Created attachment 16350 [details]
Fixes style issues

I fixed the style issues, but kept the function in gtkwebkitpage for consistency. Right now, the frame just has some accessor functions, and all the functions that act on the frame live in the page. It seems weird to do this differently, but I can change it if you still like.

[Adam Roben (:aroben)]

Comment on attachment 16350 [details]
Fixes style issues

I think ultimately we want all those functions that implicitly work on the main frame to move to WebKitGtkFrame so that callers can execute them on any frame. I think we can set a good example here by putting execute_script on WebKitGtkFrame. Otherwise everything looks good!

[Sean Egan]

I'll go ahead and move everything into Frame, then. Should I keep convenience functions in Page that call the new functions on the main Frame, or just move it all outright?

[Adam Roben (:aroben)]

(In reply to comment #7)
> I'll go ahead and move everything into Frame, then. Should I keep convenience
> functions in Page that call the new functions on the main Frame, or just move
> it all outright?

We have no such convenience methods on Mac/Windows, and I don't think they're all that necessary, given that

webkit_gtk_frame_execute_script(webkit_gtk_page_get_main_frame(page), script)

is pretty readable/simple. It also makes it clearer what's going on, especially since the DOM window object (which is the global object) is different for each frame, so the behavior of the script could be different depending on which frame it executes on.

[Sean Egan]

Created attachment 16353 [details]
Moved functions from Page to Frame

[Adam Roben (:aroben)]

Comment on attachment 16353 [details]
Moved functions from Page to Frame

Would you be willing to split the changes up into two patch (move the functions, then add the new one)? If not, I can r+ this one, but it would make for slightly cleaner history.

[Adam Roben (:aroben)]

Comment on attachment 16353 [details]
Moved functions from Page to Frame

Holger has brought it to my attention that the Mac/Windows APIs do in fact put these methods on WebView (the equivalent of WebKitGtkPage), not on WebFrame (the equivalent of WebKitGtkFrame). Given this, I think it makes sense to stick with attachment 16350 [details]. Sorry for the trouble, and thanks for sticking with it!

[Adam Roben (:aroben)]

Comment on attachment 16350 [details]
Fixes style issues

Putting this back up for review so that Holger can comment on it.

[Holger Freyther]

(In reply to comment #5)
> Created an attachment (id=16350) [edit]

As Adam pointed out I have one question/concern/remark. executeScript(WebCore::String, bool) gets invoked with executeScript(const gchar*, bool), this means auto-boxing is going to take place.

Now WebCore::String (WebCore/platform/PlatformString.h) will not do any charset conversion. So only ascii/latin1 is a valid input. I think we should follow Gtk+ examples and make all strings utf-8. So I propose adding a String::fromUtf8 around the script parameter.

[Maciej Stachowiak]

Comment on attachment 16350 [details]
Fixes style issues

r- for Holger's last comment - make sure to convert from utf8. Please resubmit when this is fixed.

[Sean Egan]

Created attachment 16492 [details]
Adds String::fromUtf8 around the script parameter, per request

Sorry for the delay; I was out of town. Feel free to make such fixes yourselves in the future ;)

[Holger Freyther]

(In reply to comment #15)
> Created an attachment (id=16492) [edit]
> Adds String::fromUtf8 around the script parameter, per request
> 
> Sorry for the delay; I was out of town. Feel free to make such fixes yourselves
> in the future ;)

Hehe, it could have been that you wanted to comment on the moving of the functions or that you wanted to have ascii/latin1 encoding for a certain reason. I think Adam is going to review this patch soon and I will land it then.

[Alp Toker]

s/programatically/programmatically but otherwise looks good.

[Alp Toker]

Comment on attachment 16492 [details]
Adds String::fromUtf8 around the script parameter, per request

s/programatically/programmatically  and maybe s/method/function

[Holger Freyther]

Landed in r26019.