Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix subscription module accessing an invalid URL for REST-API #85

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

SebDieBln
Copy link

@SebDieBln SebDieBln commented Aug 27, 2023

The subscription REST-API lives under /subscription. However the code tried accessing /undefined/subscriptionundefined.

2023-08-27 14:05:51,762 Trac[main] WARNING: [172.17.0.1]
HTTPNotFound: 404 Trac Error (No node undefined/subscriptionundefined),
<RequestWithSession "GET '/browser/undefined/subscriptionundefined'">,
referrer 'http://127.0.0.1:30443/browser/SomeRepo'

The reason is that the base URL is supposed to be encoded in the HTML attribute data-base-url and the path in data-path. These attribute names can not be used in Python as argument because they contain dashes. So in Python the dashes are replaced by underscores. genshi.builder.tag converts the underscores back to dashes and therefore generates the required attribute names. Its supposed replacement trac.util.html.html (which was enabled in fa7cdff) does not perform such a conversion thereby generating the wrong HTML attributes leading to the wrong URL in the request.

For the upcoming migration away from Genshi we could consider using the already present CodeComments dictionary. It already contains a "path" element, although with a somewhat different content.

…ible with trac.util.html.tag

genshi.builder.tag converts attribute names like 'data_base_url' to 'data-base-url' which makes them accessible by jQuery's .data() API. trac.util.html.tag does not do that.

fixes regression in fa7cdff
@SebDieBln SebDieBln marked this pull request as ready for review August 27, 2023 16:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant