Fix webhooks broken by the Backbone/underscore removal#23173
Draft
itisAliRH wants to merge 5 commits into
Draft
Conversation
Inject the webhook script only after nextTick so the `#<webhookId>` mount point has rendered. Previously appendScriptStyle ran synchronously right after setting webhookId, so the injected inline script executed before Vue flushed the v-if div and could not find its target element. Add a Webhook.vue unit test covering the ordering.
Rewrite the phdcomics webhook script without Backbone, underscore or jQuery, which were removed from the client. The script now builds its DOM directly and fetches the comic via the existing server-side helper using fetch(). DOM element ids are unchanged so styles.css still applies.
Rewrite the xkcd webhook script without Backbone, underscore or jQuery. Add an __init__.py helper that fetches a random comic from the xkcd JSON API server-side; the previous client-side JSONP call to http://dynamic.xkcd.com is blocked as mixed content on HTTPS pages and the API sends no CORS headers. DOM element ids are unchanged so styles.css still applies.
Assert that no served webhook script references Backbone, underscore or jQuery globals, which are no longer available to injected scripts.
Note that Backbone, underscore and jQuery are no longer available to injected webhook scripts and point to the ported example plugins.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Tool and workflow webhooks stopped rendering anything after Backbone and underscore were removed from the client in #22357 and #22355. The former explicitly removed "the global
window.Backboneexposure from libs.js".Webhook
script.jsfiles are injected into the page as-is and run in the global scope, so they can only use globals the client still exposes. Both example plugins were written againstBackbone.View.extendand_.template, so they now throw immediately and their container renders empty:jQuery is still assigned to
window.$/window.jQuery(client/src/entry/libs.js), which is why$-only webhooks such asgtnkept working and the breakage looks selective rather than total.What this PR does
phdcomicsandxkcdexample webhooks to self-contained vanilla JS (plain DOM APIs +fetch). Element ids are unchanged, so the existingstyles.cssstill applies.xkcdserver-side helper (__init__.py). The old script calledhttp://dynamic.xkcd.com/api-0/jsonp/comicdirectly from the browser; that endpoint now redirects to HTTPS and the xkcd JSON API sends no CORS headers, so the call is blocked as mixed content on any HTTPS Galaxy. Fetching server-side matches whatphdcomicsalready does. The helper also skips comic 404, which intentionally does not exist and returns HTTP 404, and bounds both requests with a timeout.Webhook.vue:appendScriptStyle()ran synchronously right after settingwebhookId, so the injected script executed before Vue had flushed thev-ifdiv and could not find its own#<webhookId>mount point. The old scripts happened to mask this by deferring their body through$(document).ready; vanilla scripts run immediately, so the script is now injected afternextTick().webhooks.rstthat webhook scripts must be self-contained vanilla JS.Not included
config/plugins/webhooks/demo/search/script.js(thesearchoveroverlay) is also Backbone-based and still broken. It is ~49 KB of bundled view code and isactivate: falseby default, so porting it is left to a follow-up rather than bundling it here.Webhooks maintained outside this repository (for example usegalaxy.eu's
citation_neededandiframe) are affected by the same root cause and need the equivalent change in their own repositories.How to test the changes?
(Select all options that apply)
webhooks_dir: test/functional/webhooksingalaxy.yml.ReferenceErrormessages.License