Bug 134738 - [GTK] Add support for user scripts to WebKitUserContentManager
Summary: [GTK] Add support for user scripts to WebKitUserContentManager
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKitGTK (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Adrian Perez
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-07-08 12:30 PDT by Adrian Perez
Modified: 2014-08-06 08:07 PDT (History)
9 users (show)

See Also:


Attachments
Patch (22.08 KB, patch)
2014-07-08 12:46 PDT, Adrian Perez
no flags Details | Formatted Diff | Diff
Patch (21.97 KB, patch)
2014-07-09 11:44 PDT, Adrian Perez
cgarcia: review+
cgarcia: commit-queue-
Details | Formatted Diff | Diff
Patch for landing (21.41 KB, patch)
2014-08-06 06:45 PDT, Carlos Garcia Campos
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Adrian Perez 2014-07-08 12:30:15 PDT
This would be similar as for user style sheets from bug #34551
but in this case for user scripts.
Comment 1 Adrian Perez 2014-07-08 12:46:24 PDT
Created attachment 234583 [details]
Patch
Comment 2 WebKit Commit Bot 2014-07-08 12:47:48 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 Adrian Perez 2014-07-09 11:44:14 PDT
Created attachment 234648 [details]
Patch
Comment 4 Carlos Garcia Campos 2014-08-03 03:38:35 PDT
Comment on attachment 234648 [details]
Patch

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

The API looks good to me, it's the equivalent to the style sheets API.

> Tools/TestWebKitAPI/Tests/WebKit2Gtk/TestWebKitUserContentManager.cpp:133
> -    removeOldInjectedStyleSheetsAndResetLists(test->m_userContentManager.get(), whitelist, blacklist);
> +    removeOldInjectedContentAndResetLists(test->m_userContentManager.get(), whitelist, blacklist);

Why do we need to remove also the scripts?

> Tools/TestWebKitAPI/Tests/WebKit2Gtk/TestWebKitUserContentManager.cpp:135
> +    // Without a whitelist or a blacklist all URLs should have the injected style sheet.

Looks like this method is for testing style sheets (testUserContentManagerInjectedStyleSheet), but it's testing user scripts instead.

> Tools/TestWebKitAPI/Tests/WebKit2Gtk/TestWebKitUserContentManager.cpp:171
> +static void testUserContentManagerInjectedScript(UserContentManagerTest* test, gconstpointer)

And here you are testing the style sheets . . .
Comment 5 Gustavo Noronha (kov) 2014-08-06 05:23:07 PDT
Comment on attachment 234648 [details]
Patch

API looks good to me as well =)
Comment 6 Carlos Garcia Campos 2014-08-06 06:45:37 PDT
Created attachment 236098 [details]
Patch for landing
Comment 7 WebKit Commit Bot 2014-08-06 08:07:27 PDT
Comment on attachment 236098 [details]
Patch for landing

Clearing flags on attachment: 236098

Committed r172147: <http://trac.webkit.org/changeset/172147>
Comment 8 WebKit Commit Bot 2014-08-06 08:07:33 PDT
All reviewed patches have been landed.  Closing bug.