Currently we have a debug ASSERT to ensure a URI scheme handler is not registered more than once, but nothing checks this in release builds. See: https://gitlab.gnome.org/GNOME/evolution/-/issues/1286
Be careful with the outcome, considering what it can do with older applications, where you update WebKitGTK at (without updating the affected applications), please.
Created attachment 417094 [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 https://trac.webkit.org/wiki/WebKitGTK/AddingNewWebKit2API
(In reply to Milan Crha from comment #1) > Be careful with the outcome, considering what it can do with older > applications, where you update WebKitGTK at (without updating the affected > applications), please. Thing is, there's no question what the correct behavior is: emit a critical and return early. Duplicate registrations cannot possibly work as expected and should be ignored. I guess with Evolution, the behavior you were actually getting was for the second registration to take precedence over the previous registration? If so, we're just going to have to update Evolution in tandem with WebKit, or tell distros to patch Evolution downstream. That's OK as long as we don't forget to tell distros to handle this with care.
Ping reviewers
(In reply to Michael Catanzaro from comment #5) > Ping reviewers
Comment on attachment 417094 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=417094&action=review > Source/WebKit/UIProcess/API/glib/WebKitWebContext.cpp:1242 > + if (context->priv->uriSchemeHandlers.get(scheme)) { > + g_critical("Cannot register URI scheme %s more than once", scheme); > + return; > + } I guess a g_warning should be enough, since this is not fatal in the end. Instead of this early return, we could change the "set" below by an "add" and check if addResult.isNewEntry is false.
(In reply to Carlos Garcia Campos from comment #7) > > Source/WebKit/UIProcess/API/glib/WebKitWebContext.cpp:1242 > > + if (context->priv->uriSchemeHandlers.get(scheme)) { > > + g_critical("Cannot register URI scheme %s more than once", scheme); > > + return; > > + } > > I guess a g_warning should be enough, since this is not fatal in the end. g_critical() seems like a better choice because this is undefined behavior (programmer error). > Instead of this early return, we could change the "set" below by an "add" > and check if addResult.isNewEntry is false. Sure.
Created attachment 417878 [details] Patch
commit-queue failed to commit attachment 417878 [details] to WebKit repository.
commit-queue error is: ERROR from SVN: Item is out of date: File '/trunk/Source/WebCore/ChangeLog' is out of date W: 4e3e4e0e2a88e15386956b2a49eafbeb5ac8e2cc and refs/remotes/origin/main differ, using rebase: :040000 040000 41d277ce2779148216363a2d1801423625051a8a 881cae33229fc437cba1cdd4a5f7318c45ca5f24 M Source Current branch main is up to date. ERROR: Not all changes have been committed into SVN, however the committed ones (if any) seem to be successfully integrated into the working tree. Please see the above messages for details. Not sure what's going on here. Let me rebase and try again....
P.S. Let's avoid backporting this commit.
Created attachment 417967 [details] Patch for landing
Committed r271647: <https://trac.webkit.org/changeset/271647> All reviewed patches have been landed. Closing bug and clearing flags on attachment 417967 [details].