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
Created attachment 16345 [details] allow application to call javascript on a WebkitPage
Created attachment 16347 [details] Call Javascript from gtkwebkitpage
Whoops! My first patch still had some test code in it. Removed.
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!
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.
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!
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?
(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.
Created attachment 16353 [details] Moved functions from Page to Frame
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.
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!
Comment on attachment 16350 [details] Fixes style issues Putting this back up for review so that Holger can comment on it.
(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.
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.
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 ;)
(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.
s/programatically/programmatically but otherwise looks good.
Comment on attachment 16492 [details] Adds String::fromUtf8 around the script parameter, per request s/programatically/programmatically and maybe s/method/function
Landed in r26019.