Remove non-inclusive terminology from WebKit variable nmes and test-only SPI
Created attachment 405510 [details] Patch
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 on attachment 405510 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=405510&action=review > Source/WebKit/ChangeLog:3 > + Remove non-inclusive terminology from WebKit variable nmes and test-only SPI nmes > Source/WebKit/UIProcess/API/glib/WebKitUserContent.cpp:156 > * @level: A #WebKitUserStyleLevel Need to check with GTK folks about changing their headerdoc. > Source/WebKit/WebProcess/InjectedBundle/API/c/WKBundlePrivate.h:58 > -WK_EXPORT void WKBundleAddOriginAccessWhitelistEntry(WKBundleRef bundle, WKStringRef, WKStringRef, WKStringRef, bool); > -WK_EXPORT void WKBundleRemoveOriginAccessWhitelistEntry(WKBundleRef bundle, WKStringRef, WKStringRef, WKStringRef, bool); > -WK_EXPORT void WKBundleResetOriginAccessWhitelists(WKBundleRef bundle); > +WK_EXPORT void WKBundleAddOriginAccessAllowListEntry(WKBundleRef bundle, WKStringRef, WKStringRef, WKStringRef, bool); > +WK_EXPORT void WKBundleRemoveOriginAccessAllowListEntry(WKBundleRef bundle, WKStringRef, WKStringRef, WKStringRef, bool); > +WK_EXPORT void WKBundleResetOriginAccessAllowLists(WKBundleRef bundle); If these are renameable (thus unused), shouldn't we just delete them??? (or maybe they're only used by Safari and you're going to land a change there in sync? but then what about ToT-WebKit-against-older-Safari?)
As seen later, they're only used by WebKitTestRunner, which we can change at the same time.
Created attachment 405512 [details] Patch
(In reply to Alex Christensen from comment #4) > As seen later, they're only used by WebKitTestRunner, which we can change at > the same time. Fair!
Comment on attachment 405512 [details] Patch r+ for all but the GTK port parts
Adding some GTK folks.
Comment on attachment 405512 [details] Patch r=me
Comment on attachment 405512 [details] Patch Requesting review from someone not from Apple.
Comment on attachment 405512 [details] Patch The WPE/GTK bits look fine to me: only parameter names are changed, which means the API (and ABI) remains the same—that's perfect :)
Comment on attachment 405512 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=405512&action=review > Source/WebKit/UIProcess/API/glib/WebKitUserContent.cpp:158 > - * @whitelist: (array zero-terminated=1) (allow-none): A whitelist of URI patterns or %NULL > - * @blacklist: (array zero-terminated=1) (allow-none): A blacklist of URI patterns or %NULL > + * @allow_list: (array zero-terminated=1) (allow-none): An allow_list of URI patterns or %NULL > + * @block_list: (array zero-terminated=1) (allow-none): A block_list of URI patterns or %NULL Hm, this is actually public API. Changing parameter names is not an API break in C or C++, but it is for introspection. E.g. this could be called from python; if the python code is using positional parameters, it will break. In practice, I bet no apps are doing that, so it's probably fine... but if I'm wrong, anything that is will definitely break. I guess we can try it anyway and revert if someone complains we broke their app. This API is relatively new, and it's relatively unlikely that apps are using it. Or we could take the safer approach and omit the changes to WebKitUserContent.cpp and the two public WebKitUserContent.h headers. From the title of this issue, I guess that was the intent anyway. CC other GTK devs for opinions on whether it's worth the risk.
This bug is a part of bug 213092 which implies we hope to remove all references to the terms "whitelist" and "blacklist" and many other projects are doing the same. See https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=49decddd39e5f6132ccd7d9fdc3d7c470b0061bb Since the GTK API is relatively new, that probably means that if anyone is using it with strange python bindings, it's a project that is still maintained. Since the ABI is unbroken, I would assume it's low risk.
(In reply to Alex Christensen from comment #13) > This bug is a part of bug 213092 which implies we hope to remove all > references to the terms "whitelist" and "blacklist" and many other projects > are doing the same. See > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/ > ?id=49decddd39e5f6132ccd7d9fdc3d7c470b0061bb > > Since the GTK API is relatively new, that probably means that if anyone is > using it with strange python bindings, it's a project that is still > maintained. Since the ABI is unbroken, I would assume it's low risk. User content api is not relatively new, but I'm fine with the change anyway, it's very unlikely it can break any app.
Yeah sorry, I was thinking this was the content filters API but it's actually the user stylesheets and user JS API. Calling WebKitGTK APIs from scripting languages is actually very common on Linux, but the first python app I checked (eolie) is not using these functions at all, and they are not very commonly-used APIs, so after thinking on it for a night I agree the risk is relatively low.
(In reply to Carlos Garcia Campos from comment #14) > (In reply to Alex Christensen from comment #13) > > This bug is a part of bug 213092 which implies we hope to remove all > > references to the terms "whitelist" and "blacklist" and many other projects > > are doing the same. See > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/ > > ?id=49decddd39e5f6132ccd7d9fdc3d7c470b0061bb > > > > Since the GTK API is relatively new, that probably means that if anyone is > > using it with strange python bindings, it's a project that is still > > maintained. Since the ABI is unbroken, I would assume it's low risk. > > User content api is not relatively new, but I'm fine with the change anyway, > it's very unlikely it can break any app. Also most languages which use the functions through GObject-Introspection do not support named parameters on function calls (only their positions in the call matter), and those will be fine with the change. I think the main exception is Python where you can explicitly name parameters in a function call expression: def do_something(a, b): # define a function. # ... do_something(b=1, a=2) # call it naming the parameters explicitly. do_something(2, 1) # this does the same as the previous call. I wonder how many Python applications use WebKitGTK *and* specify parameter names for the few functions changed by this patch. Definitely not Eolie, and that's probably the biggest Python application I am aware of 🤔️
Committed r265081: <https://trac.webkit.org/changeset/265081> All reviewed patches have been landed. Closing bug and clearing flags on attachment 405512 [details].
<rdar://problem/66327965>