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

Strip tags of ToC item's aria label #5536

Merged
merged 1 commit into from
May 22, 2023
Merged

Conversation

pawamoy
Copy link
Sponsor Contributor

@pawamoy pawamoy commented May 21, 2023

Some plugins/extensions might use the data-toc-label attribute of headings to set a particular string for the ToC item title.

While not explicitly documented as possible or allowed, they might set some HTML code instead of text. In this case, the aria label of Toc nav elements break the HTML as they will contain unescaped double quotes.

To fix this, we use the striptags filter to remove any tags from the aria label, keeping text only.

Copy link
Owner

@squidfunk squidfunk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! Sounds legit, as previously discussed via Gitter. Some minor adjustments.

@@ -28,7 +28,7 @@

<!-- Table of contents list -->
{% if toc_item.children %}
<nav class="md-nav" aria-label="{{ toc_item.title }}">
<nav class="md-nav" aria-label="{{ toc_item.title|striptags }}">
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please stick to our code style and insert spaces left and right to the | pipe ☺️ Also, please build the theme and make the changes in the material folder, or they won't be picked up.

@pawamoy
Copy link
Sponsor Contributor Author

pawamoy commented May 21, 2023

I ran npm install then npm run build and committed a fixup.

@squidfunk
Copy link
Owner

Thanks! You need to run npm run build:all, or the custom overrides are not built.

@pawamoy
Copy link
Sponsor Contributor Author

pawamoy commented May 21, 2023

Oh OK, this wasn't clear to me from the docs:

While this command will build all theme files, it will skip the overrides used in Material for MkDocs' own documentation which are not distributed with the theme. If you forked the theme and want to build the overrides as well, use:

I understood this as "use build:all if you have a custom fork of the theme" (I don't, I'm just sending a PR).
Maybe something simpler like "use build:all if you modified partials/overrides" would do the trick 🙂

@squidfunk
Copy link
Owner

Okay, sorry, last thing: the iconsearch index got deleted - please remove that from the commit as well. This is likely because the emoji database wasn't found in the virtual environment (we kind of assume that venv is the folder name):

const emojis$ = defer(() => resolve("venv/**/twemoji_db.py"))

This is a shortcoming of our current build process. We're going to revamp the docs and the process in the near future.

@pawamoy
Copy link
Sponsor Contributor Author

pawamoy commented May 22, 2023

Sure, no worries! Indeed I usually use .venv as folder name 🙂

Some plugins/extensions might use the data-toc-label attribute
of headings to set a particular string for the ToC item title.

While not explicitly documented as possible or allowed,
they might set some HTML code instead of text.
In this case, the aria label of Toc nav elements break the HTML
as they will contain unescaped double quotes.

To fix this, we use the `striptags` filter
to remove any tags from the aria label, keeping text only.
@squidfunk
Copy link
Owner

Perfect, good to go now!

@squidfunk squidfunk merged commit 263a85d into squidfunk:master May 22, 2023
3 checks passed
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

2 participants