fix(build): check for bad_src flaws in markdown files #8133

yin1999 · 2023-02-05T02:07:21Z

Summary

As we are 100% markdown now, but the images check will only match src in HTML files (see function findMatchesInText).

And we already have a function findMatchesInMarkdown, which is used to match links in markdown. I just reuse this function to match image's src. So we could show bad src flaws for markdown files.

Also as a part of #7574, for the bad_image test will be failed after convert testing files to markdown.

Problem

The bad_src flaws for image files are only works for html files. So we couldn't be notified when there is a bad image src within a markdown document.

Solution

check whether the document is a markdown file. If so, use findMatchesInMarkdown to match image's src.

To make this be a test, convert the bad_image test file to markdown (as we are 100% markdown now).

Screenshots

Try to add a image (with bad image src) in document: files/zh-cn/mdn/at_ten/index.md

Then, run yarn dev and open http://localhost:5042/zh-CN/docs/MDN/At_ten/index.json

Before

We could not see the bad_src flaws

After

We could see the bad_src flaws

How did you test this change?

run yarn dev and yarn test

build/matches-in-text.ts

yin1999 · 2023-02-06T06:57:13Z

build/check-images.ts

+    const matches = isMarkdown
+      ? findMatchesInMarkdown(rawContent, "image", src)
+      : findMatchesInText(src, rawContent, {
+          attribute: "src",
+        });


As we still have many HTML tables in content. Is it necessary to match both in Markdown and Text for markdown files? (but we may need to ignore the html codes in <pre>

example: mdn/content#24154

Yes, indeed, the HTML table on the cursor page is not going away anytime soon, so we should also findMatchesInText() for Markdown files.

Hi @caugner, as we need to sort the matches, so the codes might be:

const matches = findMatchesInText(src, rawContent, { attribute: "src", }); if (isMarkdown) { const hasMatchInHTML = matches.length > 0; matches.push( ...findMatchesInMarkdown(src, rawContent, { type: "image" }) ); // note that, we need to sort them by line and column if (hasMatchInHTML) { matches.sort((a, b) => a.line === b.line ? a.column - b.column : a.line - b.line ); } }

I'm not sure could we call findMatchesInText function in findMatchesInMarkdown as we could deal with broken html format links in markdown file.

Found that the fromMarkdown can also parse html nodes: see AST.

Resolved this by also parse HTML node in markdown

caugner

LGTM, just some nits.

build/flaws/broken-links.ts

build/matches-in-text.ts

caugner · 2023-02-06T17:59:00Z

build/check-images.ts

+    const matches = isMarkdown
+      ? findMatchesInMarkdown(rawContent, "image", src)
+      : findMatchesInText(src, rawContent, {
+          attribute: "src",
+        });


Yes, indeed, the HTML table on the cursor page is not going away anytime soon, so we should also findMatchesInText() for Markdown files.

testing/content/files/en-us/web/images/bad_src/index.md

build/check-images.ts

yin1999 · 2023-02-08T02:29:36Z

testing/tests/index.test.ts

@@ -1070,6 +1070,21 @@ test("image flaws with bad images", () => {
        "File not present on disk, an empty file, or not an image"
    ).length
  ).toBe(4);
+  // Check the line and column numbers for html <img> tags in markdown


It seems that the start column of markdown images are the index of ! (+1). Only add line and column test for html elements

yin1999 · 2023-02-08T04:50:57Z

Trying to fix the testing error in mdn/content#24265

github-actions · 2023-02-08T12:29:39Z