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(ci/markdownlint): add a search-replace rule to put some html tags in code spans #36110

Closed
wants to merge 2 commits into from

Conversation

OnkarRuikar
Copy link
Contributor

We are currently using backslash (\<audio>) to allow HTML tags in markdown text. It would be better to use markdown code spans(`<audio>`) instead.

I added the search-replace rule and ran yarn fix:md command to fix the content files.

cc @caugner

@OnkarRuikar OnkarRuikar requested review from mdn-bot and a team as code owners September 30, 2024 06:30
@OnkarRuikar OnkarRuikar requested review from wbamberg and chrisdavidmills and removed request for a team September 30, 2024 06:30
@github-actions github-actions bot added Content:HTML Hypertext Markup Language docs Content:WebAPI Web API docs Content:Learn Learning area docs Content:Media Media docs Content:Performance Web performance docs Content:Learn:JavaScript Learning area JavaScript docs Content:Meta Content in the meta docs Content:Learn:HTML Learning area HTML docs Content:Learn:Forms Learning area Forms docs Content:Learn:Cross-Browser-Testing Learning area Cross-Browser-Testing docs system [PR only] Infrastructure and configuration for the project size/s [PR only] 6-50 LoC changed labels Sep 30, 2024
Copy link
Contributor

github-actions bot commented Sep 30, 2024

Preview URLs (16 pages)
Flaws (9)

Note! 14 documents with no flaws that don't need to be listed. 🎉

URL: /en-US/docs/Learn/Learning_and_getting_help
Title: Learning and getting help
Flaw count: 1

  • broken_links:
    • Can't resolve /en-US/play

URL: /en-US/docs/Web/API/HTMLMediaElement
Title: HTMLMediaElement
Flaw count: 8

  • macros:
    • /en-US/docs/Web/API/HTMLMediaElement/played does not exist
    • /en-US/docs/Web/API/HTMLMediaElement/seeking does not exist
    • /en-US/docs/Web/API/MediaController does not exist
    • /en-US/docs/Web/API/MediaController does not exist
    • /en-US/docs/Web/API/HTMLMediaElement/mozAudioCaptured does not exist
    • and 3 more flaws omitted
External URLs (1)

URL: /en-US/docs/Learn/Tools_and_testing/Cross_browser_testing/Feature_detection
Title: Implementing feature detection

(comment last updated: 2024-10-01 06:20:50)

Copy link
Contributor

@chrisdavidmills chrisdavidmills left a comment

Choose a reason for hiding this comment

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

This seems reasonable to me, @OnkarRuikar, it is usually what I do in titles, etc.

But I don't know much about markdown lint, so I'll defer approving to someone else.

@OnkarRuikar
Copy link
Contributor Author

This was brought up/requested by Claas on Discord. We'll have to wait for his views.

@Josh-Cena Josh-Cena requested review from caugner and removed request for mdn-bot, a team and wbamberg October 1, 2024 05:10
@@ -367,7 +367,7 @@ The following example shows simple usage of the `<audio>` element to play an OGG

For details on when autoplay works, how to get permission to use autoplay, and how and when it's appropriate to use autoplay, see our [autoplay guide](/en-US/docs/Web/Media/Autoplay_guide).

### \<audio> element with \<source> element
### `<audio>` element with \<source> element
Copy link
Member

Choose a reason for hiding this comment

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

You've only replaced one

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have yet to learn about the broader purpose of this. But looks like we'll have to handle a lot of escaped tags. Let me know how to proceed.

\<a>
\<absoluteIRI>
\<all_urls>
\<angle>
\<anything>
\<App>
\<aside>
\<b>
\<base>
\<body>
\<BOM>
\<bookinstance_id>
\<br>
\<caption>
\<client>
\<code>
\<col>
\<color>
\<Color>
\<comment>
\<coordinate>
\<CR>
\<credentials>
\<dasharray>
\<day>
\<em>
\<email>
\<embed>
\<endpoint>
\<etag>
\<etag_value>
\<FF>
\<fieldset>
\<form>
\<frequency>
\<funciri>
\<FuncIRI>
\<g>
\<h1>
\<h2>
\<head>
\<host>
\<hour>
\<hr>
\<i>
\<icccolor>
\<id>
\<iframe>
\<img>
\<integer>
\<IRI>
\<key>
\<label>
\<legend>
\<length>
\<LF>
\<link>
\<locale>
\<LS>
\<math>
\<menu>
\<menulist>
\<menupopup>
\<method>
\<mfrac>
\<mi>
\<minute>
\<mo>
\<month>
\<mroot>
\<mrow>
\<msqrt>
\<myusername>
\<name>
\<NBSP>
\<none>
\<number>
\<Number>
\<object>
\<opacity>
\<origin>
\<paint>
\<parsererror>
\<path>
\<path_data>
\<percentage>
\<policyName>
\<port>
\<position>
\<pre>
\<product>
\<protocol>
\<proxy1>
\<proxy2>
\<PS>
\<pseudonym>
\<realm>
\<region>
\<relativeIRI>
\<response>
\<script>
\<second>
\<size>
\<something>
\<source>
\<SP>
\<string>
\<strong>
\<style>
\<svg>
\<switch>
\<TAB>
\<tbody>
\<template>
\<template_extension>
\<tfoot>
\<th>
\<thead>
\<time>
\<Todo>
\<track>
\<treecol>
\<type>
\<u>
\<unit>
\<uri>
\<url>
\<URL>
\<USP>
\<value>
\<version>
\<VT>
\<year>
\<ZWJ>
\<ZWNBSP>
\<ZWNJ>

Copy link
Member

@Josh-Cena Josh-Cena Oct 1, 2024

Choose a reason for hiding this comment

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

Strong -1 on escaping all them. "<ZWJ>" etc. are Unicode transcriptions and should not be code-formatted. IMO CSS types should not either but we already do, so let's do that consistently. Any code entity such as HTML/XML/JSX tags should indeed be code formatted.

At the end of the day I consider this to have a net 0 benefit though. We are just changing our content to please some engineering limitation.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We need to discuss this in the content call.

We are just changing our content to please some engineering limitation.

I see. Input sanitization is much better than restraining the content.

@OnkarRuikar
Copy link
Contributor Author

OnkarRuikar commented Oct 1, 2024

How about we convert to &lt;audio> instead of `<audio>`?

@github-actions github-actions bot added the merge conflicts 🚧 [PR only] label Oct 25, 2024
Copy link
Contributor

This pull request has merge conflicts that must be resolved before it can be merged.

@caugner
Copy link
Contributor

caugner commented Oct 28, 2024

@OnkarRuikar Thank you for opening this PR, and sorry for not getting back to you earlier.

I allowed myself to cherry-pick (and extend) your change here: mdn/translated-content-de@fb7a454

@OnkarRuikar OnkarRuikar deleted the ci_md_lint_rule branch October 29, 2024 03:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Content:HTML Hypertext Markup Language docs Content:Learn:Cross-Browser-Testing Learning area Cross-Browser-Testing docs Content:Learn:Forms Learning area Forms docs Content:Learn:HTML Learning area HTML docs Content:Learn:JavaScript Learning area JavaScript docs Content:Learn Learning area docs Content:Media Media docs Content:Meta Content in the meta docs Content:Performance Web performance docs Content:WebAPI Web API docs merge conflicts 🚧 [PR only] size/s [PR only] 6-50 LoC changed system [PR only] Infrastructure and configuration for the project
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants