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

Bump antsibull version from 0.40.1 to 0.41.0 and ansible sphinx theme version to fix HTML generation in return value tables #76759

Merged
merged 4 commits into from
Feb 3, 2022

Conversation

felixfontein
Copy link
Contributor

SUMMARY

Follow-up PR to #76675. Most important change in antsibull 0.40.2 is ansible-community/antsibull#387 which fixes broken HTML in return value tables. While most browsers simply ignored it and rendered the result fine, lynx wouldn't render the result.

ISSUE TYPE
  • Docs Pull Request
COMPONENT NAME

docs build

@ansibot ansibot added affects_2.13 core_review In order to be merged, this PR must follow the core review workflow. docs This issue/PR relates to or includes documentation. needs_triage Needs a first human triage before being processed. small_patch support:core This issue/PR relates to code supported by the Ansible Engineering Team. test This PR relates to tests. labels Jan 14, 2022
@felixfontein felixfontein changed the title Bump antsibull version from 0.40.1 to 0.40.2 to fix HTML generation in return value tables [WIP] Bump antsibull version from 0.40.1 to 0.40.2 to fix HTML generation in return value tables Jan 14, 2022
@felixfontein
Copy link
Contributor Author

The Sphinx theme also generates invalid HTML; will be fixed by ansible-community/sphinx_ansible_theme#44.

@ansibot ansibot added WIP This issue/PR is a work in progress. Nevertheless it was shared for getting input from peers. and removed core_review In order to be merged, this PR must follow the core review workflow. labels Jan 14, 2022
@mkrizek mkrizek removed the needs_triage Needs a first human triage before being processed. label Jan 18, 2022
@felixfontein
Copy link
Contributor Author

Ok, the generated HTML should be correct (or at least have a lot less errors than before). But now that we waited so long, we can also wait another week until ansible-community/community-topics#61 is resolved, and include a new antsibull release. That one will also support the new keyword feature and remove email addresses from author lines.

I think it's worth the wait. @samccann what do you think?

@samccann
Copy link
Contributor

samccann commented Jan 26, 2022

Yes let's wait before bumping the antsibull version. That way I can test both changes locally before we merge this PR.

@felixfontein felixfontein changed the title [WIP] Bump antsibull version from 0.40.1 to 0.40.2 to fix HTML generation in return value tables Bump antsibull version from 0.40.1 to 0.40.2 to fix HTML generation in return value tables Feb 1, 2022
@felixfontein
Copy link
Contributor Author

ready_for_review

@ansibot ansibot added core_review In order to be merged, this PR must follow the core review workflow. and removed WIP This issue/PR is a work in progress. Nevertheless it was shared for getting input from peers. labels Feb 1, 2022
@felixfontein felixfontein changed the title Bump antsibull version from 0.40.1 to 0.40.2 to fix HTML generation in return value tables Bump antsibull version from 0.40.1 to 0.41.0 and ansible sphinx theme version to fix HTML generation in return value tables Feb 1, 2022
@felixfontein
Copy link
Contributor Author

I did check the W3C validator and Lynx on the resulting pages (concretely: http://docs.testing.ansible.com/ansible/devel/collections/community/hashi_vault/hashi_vault_lookup.html#ansible-collections-community-hashi-vault-hashi-vault-lookup) and they were still not happy. Commit 3030dbb removes one further warning from the validator (if you want to see why this was a warning and what kind of abysses can open up: https://www.howtocreate.co.uk/SGMLComments.html#doubledash).

Lynx is still complaining because there's a <select> outside a <form>: https://github.com/ansible-community/sphinx_ansible_theme/blob/master/sphinx_ansible_theme/version_chooser.html#L16 From the HTML 5 standard (https://html.spec.whatwg.org/multipage/form-elements.html#the-select-element, https://html.spec.whatwg.org/multipage/form-control-infrastructure.html#form-owner, https://html.spec.whatwg.org/multipage/forms.html#form-associated-element) it does not look to me that this is actually a HTML 5 requirement. I'll create an issue in the theme's repo for that where we can discuss whether to fix it (and if yes, how).

@samccann samccann merged commit bc1e29d into ansible:devel Feb 3, 2022
@samccann
Copy link
Contributor

samccann commented Feb 3, 2022

@felixfontein thanks!

@felixfontein felixfontein deleted the bump-antsibull-version branch February 3, 2022 19:19
@felixfontein
Copy link
Contributor Author

@samccann thanks for testing, reviewing and merging :)

bcoca pushed a commit to bcoca/ansible that referenced this pull request Feb 7, 2022
… version to fix HTML generation in return value tables (ansible#76759)


* Use Jinja2 comment instead of HTML comment to avoid W3C validator warning (-- inside comment).

(For those interested in the history of this: http://www.howtocreate.co.uk/SGMLComments.html#doubledash)
@ansible ansible locked and limited conversation to collaborators Feb 17, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
affects_2.13 core_review In order to be merged, this PR must follow the core review workflow. docs This issue/PR relates to or includes documentation. has_issue support:core This issue/PR relates to code supported by the Ansible Engineering Team. test This PR relates to tests.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants