From 4fd6331f77a17478f40ee8497df316aff99bce6b Mon Sep 17 00:00:00 2001 From: Florian Dieminger Date: Tue, 29 Oct 2024 20:50:07 +0100 Subject: [PATCH] fix(various): small fixes from diffing with rari (#12045) * Localize "page-not-created" title. * Don't put .html files into history. * fix html of GlossarySidebar (it had
    . * strip https://developer.mozilla.org from links. * use href instead of the deprecated xlink:href. * ... -> ... to be consistent. * reuse PreviousMenuNext for PreviousNext macro. * Consistent attribute order. * Don't inject id's into divs anymore. * Warn on fallback for code collection for live samples (if there's no header found). * Always remove href from "page-not-created" links. * Sort history entries. --- build/flaws/broken-links.ts | 11 +- build/git-history.ts | 5 +- build/utils.ts | 4 + kumascript/macros/GlossarySidebar.ejs | 2 + kumascript/macros/InheritanceDiagram.ejs | 2 +- kumascript/macros/MathMLElement.ejs | 2 +- kumascript/macros/PreviousNext.ejs | 41 +------ kumascript/macros/ReadOnlyInline.ejs | 2 +- kumascript/macros/SVGAttr.ejs | 2 +- kumascript/src/api/util.ts | 14 +-- .../macros/fixtures/apiref/commonl10n.json | 7 ++ kumascript/tests/macros/svgattr.test.ts | 2 +- .../css_layout/introduction/flex/index.html | 5 +- .../css/css_layout/introduction/index.html | 29 ++--- testing/tests/headless.index.spec.ts | 113 ------------------ testing/tests/index.test.ts | 4 +- tool/cli.ts | 18 ++- 17 files changed, 73 insertions(+), 190 deletions(-) diff --git a/build/flaws/broken-links.ts b/build/flaws/broken-links.ts index 30f53aff04a5..487415c354b7 100644 --- a/build/flaws/broken-links.ts +++ b/build/flaws/broken-links.ts @@ -12,6 +12,8 @@ import * as cheerio from "cheerio"; import { Doc } from "../../libs/types/document.js"; import { Flaw } from "./index.js"; import { ONLY_AVAILABLE_IN_ENGLISH } from "../../libs/l10n/l10n.js"; +import web from "../../kumascript/src/api/web.js"; +import mdn from "../../kumascript/src/api/mdn.js"; const _safeToHttpsDomains = new Map(); @@ -62,7 +64,14 @@ function mutateLink( $element.attr("href", suggestion); } else { $element.addClass("page-not-created"); - $element.attr("title", "This is a link to an unwritten page"); + const locale = $element.attr("href")?.match(/^\/([^/]+)\//)?.[1] || "en-US"; + const titleWhenMissing = (mdn as any).getLocalString.call( + { env: { locale } }, + web.getJSONData("L10n-Common"), + "summary" + ); + $element.attr("title", titleWhenMissing); + $element.attr("href", null); } } diff --git a/build/git-history.ts b/build/git-history.ts index 8386c43a16f7..ebc991db4731 100644 --- a/build/git-history.ts +++ b/build/git-history.ts @@ -80,10 +80,7 @@ export function gather(contentRoots, previousFile = null) { // the git root. For example "../README.md" and since those aren't documents // exclude them. // We also only care about documents. - if ( - !key.startsWith(".") && - (key.endsWith("index.html") || key.endsWith("index.md")) - ) { + if (!key.startsWith(".") && key.endsWith("index.md")) { map.set(key, Object.assign(value, { merged: parents.get(value.hash) })); } } diff --git a/build/utils.ts b/build/utils.ts index 9a92f646ee07..d2874dd0cee1 100644 --- a/build/utils.ts +++ b/build/utils.ts @@ -196,6 +196,10 @@ export function postProcessExternalLinks($) { // But we haven't applied all fixable flaws yet and we still have to // support translated content which is quite a long time away from // being entirely treated with the fixable flaws cleanup. + $a.attr( + "href", + $a.attr("href").replace("https://developer.mozilla.org", "") || "/" + ); return; } $a.addClass("external"); diff --git a/kumascript/macros/GlossarySidebar.ejs b/kumascript/macros/GlossarySidebar.ejs index e42808ac8d1d..2d188ab2fbd7 100644 --- a/kumascript/macros/GlossarySidebar.ejs +++ b/kumascript/macros/GlossarySidebar.ejs @@ -21,6 +21,8 @@ async function getPageLinkAndTitle(slug) { diff --git a/kumascript/macros/InheritanceDiagram.ejs b/kumascript/macros/InheritanceDiagram.ejs index b237e8cc57cb..1dc8a91aa639 100644 --- a/kumascript/macros/InheritanceDiagram.ejs +++ b/kumascript/macros/InheritanceDiagram.ejs @@ -40,7 +40,7 @@ function rectWithText(x, y, fill, interfaceName, reverse) { const rectWidth = calculateRectWidth(interfaceName); if (reverse) x -= rectWidth; return ` - + ${interfaceName} diff --git a/kumascript/macros/MathMLElement.ejs b/kumascript/macros/MathMLElement.ejs index 2867cb1a0a11..a54f8944dbab 100644 --- a/kumascript/macros/MathMLElement.ejs +++ b/kumascript/macros/MathMLElement.ejs @@ -7,6 +7,6 @@ var sectionname = mdn.localString({ }); var dest = '/' + env.locale + '/docs/Web/MathML/' + sectionname + '/' + name; -var result = '<' + name + '>'; +var result = '<' + name + '>'; %> <%- result %> diff --git a/kumascript/macros/PreviousNext.ejs b/kumascript/macros/PreviousNext.ejs index 40977b4c5963..9d87e36c6cf4 100644 --- a/kumascript/macros/PreviousNext.ejs +++ b/kumascript/macros/PreviousNext.ejs @@ -1,40 +1,5 @@ <% -/* - -Parameter - $0 (first parameter): path of Previous page - $1 (second parameter): path of Next page - -Issue - * Problem of Apostrophe (https://developer.mozilla.org/fr/docs/JavaScript_Guide/Op%C3%A9rateurs/Op%C3%A9rateurs_sp%C3%A9ciaux) - -*/ - -var lang = env.locale; -var strPrevious = ""; -var strNext = ""; - -var s_PreviousNext = mdn.localString({ - "en-US": ["« Previous", "Next »"], - "es" : ["« Anterior", "Siguiente »"], - "fr" : ["« Précédent", "Suivant »"], - "ja" : ["« 前のページ", "次のページ »"], - "ko" : ["« 이전", "다음 »"], - "ru" : ["« Предыдущая статья", "Следующая статья »"], - "zh-CN": ["« 上一页", "下一页 »"], - "zh-TW": ["« 前頁", "次頁 »"] -}); - - -if ($0) { - strPrevious = '
  1. ' + s_PreviousNext[0] + '
    2. '; -} - -if ($1) { - strNext = '
  2. ' + s_PreviousNext[1] + '
    3. '; -} +/* two parameters: path of previous page and path to next */ +/* Calls PreviousMenuNext to avoid duplication of translations */ %> - +<%- await template("PreviousMenuNext", [$0, $1]) %> diff --git a/kumascript/macros/ReadOnlyInline.ejs b/kumascript/macros/ReadOnlyInline.ejs index 3aa43a078e1b..a10ff066958f 100644 --- a/kumascript/macros/ReadOnlyInline.ejs +++ b/kumascript/macros/ReadOnlyInline.ejs @@ -18,4 +18,4 @@ var title = mdn.localString({ "zh-CN": "该属性的值无法更改", }); %> -<%= str %> +<%= str %> diff --git a/kumascript/macros/SVGAttr.ejs b/kumascript/macros/SVGAttr.ejs index c8f0c8ca2432..f71af844c142 100644 --- a/kumascript/macros/SVGAttr.ejs +++ b/kumascript/macros/SVGAttr.ejs @@ -6,4 +6,4 @@ var slug = mdn.localString({ var URL = "/" + env.locale + "/docs/Web/SVG/" + slug + "/" + $0; %> -<%= $0 %> +<%= $0 %> diff --git a/kumascript/src/api/util.ts b/kumascript/src/api/util.ts index 3a5655b4fcb4..d88bec191bae 100644 --- a/kumascript/src/api/util.ts +++ b/kumascript/src/api/util.ts @@ -4,15 +4,11 @@ */ import sanitizeFilename from "sanitize-filename"; import * as cheerio from "cheerio"; +import chalk from "chalk"; const H1_TO_H6_TAGS = new Set(["h1", "h2", "h3", "h4", "h5", "h6"]); const HEADING_TAGS = new Set([...H1_TO_H6_TAGS, "hgroup"]); -const INJECT_SECTION_ID_TAGS = new Set([ - ...HEADING_TAGS, - "section", - "div", - "dt", -]); +const INJECT_SECTION_ID_TAGS = new Set([...HEADING_TAGS, "section", "dt"]); const LIVE_SAMPLE_PARTS = ["html", "css", "js"]; const SECTION_ID_DISALLOWED = /["#$%&+,/:;=?@[\]^`{|}~')(\\]/g; @@ -130,7 +126,6 @@ function collectClosestCode($start) { ]; }); if (pairs.some(([, code]) => !!code)) { - $start.prop("title", $level.first(":header").text()); return Object.fromEntries(pairs); } } @@ -316,6 +311,11 @@ export class HTMLTool { // We're here because we can't find the sectionID, so instead we're going // to find the live-sample iframe by its id (iframeID, NOT sectionID), and // then collect the closest blocks of code for the live sample. + console.warn( + chalk.yellow( + `invalid header id in live sample ${sectionID} within ${this.pathDescription}` + ) + ); result = collectClosestCode(findSectionStart(this.$, iframeID)); if (!result) { throw new KumascriptError( diff --git a/kumascript/tests/macros/fixtures/apiref/commonl10n.json b/kumascript/tests/macros/fixtures/apiref/commonl10n.json index 722f9c465777..99281c379893 100644 --- a/kumascript/tests/macros/fixtures/apiref/commonl10n.json +++ b/kumascript/tests/macros/fixtures/apiref/commonl10n.json @@ -102,5 +102,12 @@ "fr": "Interfaces", "ja": "インターフェイス", "es": "Interfaces" + }, + + "summary": { + "en-US": "The documentation about this has not yet been written; please consider contributing!", + "fr": "Cette documentation n'a pas encore été rédigée, vous pouvez aider en contribuant!", + "ja": "この項目についての文書はまだ書かれていません。書いてみませんか？", + "es": "La documentación acerca de este tema no ha sido escrita todavía. ¡Por favor considera contribuir!" } } diff --git a/kumascript/tests/macros/svgattr.test.ts b/kumascript/tests/macros/svgattr.test.ts index 5d2618a1b61e..206c6777d04f 100644 --- a/kumascript/tests/macros/svgattr.test.ts +++ b/kumascript/tests/macros/svgattr.test.ts @@ -7,7 +7,7 @@ describeMacro("SVGAttr", () => { macro.ctx.env.locale = locale; return assert.eventually.equal( macro.call(attr), - `${attr}` + `${attr}` ); }); } diff --git a/testing/content/files/en-us/learn/css/css_layout/introduction/flex/index.html b/testing/content/files/en-us/learn/css/css_layout/introduction/flex/index.html index 1c9b765f63a6..2d78a5d6cf6b 100644 --- a/testing/content/files/en-us/learn/css/css_layout/introduction/flex/index.html +++ b/testing/content/files/en-us/learn/css/css_layout/introduction/flex/index.html @@ -9,6 +9,7 @@ - Learn - flexbox --- +

    Flexbox

    -
    {{ Page("Learn/CSS/CSS_layout/Introduction/Flex/Stuff", "Flex_1") }}
    -

    {{ EmbedLiveSample('Flex_1', '300', '200') }}

    -
    {{ Page("Learn/CSS/CSS_layout/Introduction/Flex/Stuff", "Flex_2") }}
    > -

    {{ EmbedLiveSample('Flex_2', '300', '200') }}

    diff --git a/testing/content/files/en-us/learn/css/css_layout/introduction/index.html b/testing/content/files/en-us/learn/css/css_layout/introduction/index.html index cc189398374d..e037208de46c 100644 --- a/testing/content/files/en-us/learn/css/css_layout/introduction/index.html +++ b/testing/content/files/en-us/learn/css/css_layout/introduction/index.html @@ -15,6 +15,7 @@ - flexbox - flow --- +
    {{LearnSidebar}}
    -

    Flexbox

    -

    {{ EmbedLiveSample('Flex_1', '300', '200', "", "Learn/CSS/CSS_layout/Introduction/Flex") }}

    -

    {{ EmbedLiveSample('Flex_2', '300', '200', "", "Learn/CSS/CSS_layout/Introduction/Flex") }}

    -

    Grid Layout

    -

    {{ EmbedLiveSample('Grid_1', '300', '330', "", "Learn/CSS/CSS_layout/Introduction/Grid") }}

    - -
    .wrapper {
+  
    +.wrapper {
     display: grid;
     grid-template-columns: 1fr 1fr 1fr;
     grid-template-rows: 100px 100px;
@@ -64,14 +62,17 @@ 
    Grid example 2
    
     grid-row: 2;
     grid-column: 3;
 }
-
    
+
    -
    <div class="wrapper">
+  
    +<div class="wrapper">
     <div class="box1">One</div>
     <div class="box2">Two</div>
     <div class="box3">Three</div>
 </div>
-
    
+
    -

    {{ EmbedLiveSample('Grid_2', '300', '330') }}

    +

    {{ EmbedLiveSample('Grid_Layout', '300', '330') }}

    diff --git a/testing/tests/headless.index.spec.ts b/testing/tests/headless.index.spec.ts index 5f3a631911ea..3e441e486849 100644 --- a/testing/tests/headless.index.spec.ts +++ b/testing/tests/headless.index.spec.ts @@ -7,16 +7,6 @@ function testURL(pathname = "/") { return `http://localhost:${PORT}${pathname}`; } -function liveSampleURL(uri: string, id: string, legacy = false) { - const params = new URLSearchParams([ - ["id", id], - ...(legacy ? [["url", uri]] : []), - ]); - return `${uri}/${ - legacy ? `_sample_.${id}` : "runner" - }.html?${params.toString()}`; -} - test.describe("Basic viewing of functional pages", () => { test("open the /en-US/docs/Web/Foo page", async ({ page }) => { await page.goto(testURL("/en-US/docs/Web/Foo")); @@ -44,109 +34,6 @@ test.describe("Basic viewing of functional pages", () => { ).toBeTruthy(); }); - test("open the /en-US/docs/Learn/CSS/CSS_layout/Introduction page", async ({ - page, - }) => { - const uri = "/en-US/docs/Learn/CSS/CSS_layout/Introduction"; - const flexSample1Uri = liveSampleURL(`${uri}/Flex`, "flex_1", true); - const flexSample2Uri = liveSampleURL(`${uri}/Flex`, "flex_2", true); - const gridSample1Uri = liveSampleURL(`${uri}/Grid`, "grid_1", true); - const gridSample2Uri = liveSampleURL(uri, "grid_2"); - await page.goto(testURL(uri)); - expect(await page.title()).toContain("A Test Introduction to CSS layout"); - expect(await page.innerText("h1")).toBe( - "A Test Introduction to CSS layout" - ); - expect(await page.innerText("#flexbox")).toBe("Flexbox"); - expect( - await page.isVisible(`iframe.sample-code-frame[src$="${flexSample1Uri}"]`) - ).toBeTruthy(); - expect( - await page.isVisible(`iframe.sample-code-frame[src$="${flexSample2Uri}"]`) - ).toBeTruthy(); - expect(await page.innerText("#grid_layout")).toBe("Grid Layout"); - expect( - await page.isVisible(`iframe.sample-code-frame[src$="${gridSample1Uri}"]`) - ).toBeTruthy(); - expect( - await page.innerText("#grid_2 pre.css.notranslate:not(.hidden)") - ).toMatch(/\.wrapper\s*\{\s*display:\s*grid;/); - expect( - await page.isVisible(`iframe.sample-code-frame[src$="${gridSample2Uri}"]`) - ).toBeTruthy(); - - // Ensure that the legacy live-sample pages were built. - for (const sampleUri of [flexSample1Uri, flexSample2Uri, gridSample1Uri]) { - await page.goto(testURL(sampleUri)); - expect(await page.innerText("body > div.wrapper > div.box1")).toBe("One"); - expect(await page.innerText("body > div.wrapper > div.box2")).toBe("Two"); - expect(await page.innerText("body > div.wrapper > div.box3")).toBe( - "Three" - ); - } - }); - - test("open the /en-US/docs/Learn/CSS/CSS_layout/Introduction/Flex page", async ({ - page, - }) => { - const uri = "/en-US/docs/Learn/CSS/CSS_layout/Introduction/Flex"; - const flexSample1Uri = liveSampleURL(uri, "flex_1"); - const flexSample2Uri = liveSampleURL(uri, "flex_2"); - await page.goto(testURL(uri)); - expect(await page.title()).toContain( - "A Test Introduction to CSS Flexbox Layout" - ); - expect(await page.innerText("h1")).toBe( - "A Test Introduction to CSS Flexbox Layout" - ); - expect(await page.innerText("#flexbox")).toBe("Flexbox"); - - expect( - await page.innerText("#flex_1 pre.css.notranslate:not(.hidden)") - ).toMatch(/\.wrapper\s*\{\s*display:\s*flex;\s*\}/); - expect( - await page.isVisible(`iframe.sample-code-frame[src$="${flexSample1Uri}"]`) - ).toBeTruthy(); - - expect( - await page.innerText("#flex_2 pre.css.notranslate:not(.hidden)") - ).toMatch( - /\.wrapper {\s*display: flex;\s*\}\s*\.wrapper > div \{\s*flex: 1;\s*\}/ - ); - expect( - await page.isVisible(`iframe.sample-code-frame[src$="${flexSample2Uri}"]`) - ).toBeTruthy(); - }); - - test("open the /en-US/docs/Learn/CSS/CSS_layout/Introduction/Grid page", async ({ - page, - }) => { - const uri = "/en-US/docs/Learn/CSS/CSS_layout/Introduction/Grid"; - const gridSample1Uri = liveSampleURL(uri, "grid_1"); - const gridSample2Uri = liveSampleURL(uri, "grid_2"); - await page.goto(testURL(uri)); - expect(await page.title()).toContain( - "A Test Introduction to CSS Grid Layout" - ); - expect(await page.innerText("h1")).toBe( - "A Test Introduction to CSS Grid Layout" - ); - expect(await page.innerText("#grid_layout")).toBe("Grid Layout"); - expect( - await page.innerText("#grid_1 pre.css.notranslate:not(.hidden)") - ).toMatch(/\.wrapper\s*\{\s*display:\s*grid;/); - expect( - await page.isVisible(`iframe.sample-code-frame[src$="${gridSample1Uri}"]`) - ).toBeTruthy(); - - expect( - await page.innerText("#grid_2 pre.css.notranslate:not(.hidden)") - ).toMatch(/grid-template-columns: 1fr 1fr 1fr;/); - expect( - await page.isVisible(`iframe.sample-code-frame[src$="${gridSample2Uri}"]`) - ).toBeTruthy(); - }); - test("return to previous page on back-button press", async ({ page }) => { await page.goto(testURL("/en-US/docs/Web/Foo")); expect(await page.title()).toContain(": A test tag"); diff --git a/testing/tests/index.test.ts b/testing/tests/index.test.ts index 44937ad1f86c..dfca7fe24d0d 100644 --- a/testing/tests/index.test.ts +++ b/testing/tests/index.test.ts @@ -926,8 +926,8 @@ test("check built flaws for /en-us/learn/css/css_layout/introduction/flex page", const htmlFile = path.join(builtFolder, "index.html"); const html = fs.readFileSync(htmlFile, "utf-8"); const $ = cheerio.load(html); - // The css_layout/introduction/flex page has 2 iframes - expect($('iframe[loading="lazy"]')).toHaveLength(2); + // The css_layout/introduction/flex page has 0 iframes + expect($('iframe[loading="lazy"]')).toHaveLength(0); }); test("detect bad_bcd_queries flaws", () => { diff --git a/tool/cli.ts b/tool/cli.ts index 19d7bca1e4dc..215f189aacfb 100644 --- a/tool/cli.ts +++ b/tool/cli.ts @@ -641,15 +641,29 @@ program } let filesWritten = 0; for (const [locale, history] of Object.entries(historyPerLocale)) { + const sorted = [...Object.entries(history)]; + sorted.sort(([a], [b]) => { + if (a > b) { + return 1; + } + if (a < b) { + return -1; + } + return 0; + }); const root = getRoot(locale); const outputFile = path.join(root, locale, "_githistory.json"); - fs.writeFileSync(outputFile, JSON.stringify(history, null, 2), "utf-8"); + fs.writeFileSync( + outputFile, + JSON.stringify(Object.fromEntries(sorted), null, 2), + "utf-8" + ); filesWritten += 1; if (verbose) { console.log( chalk.green( `Wrote '${locale}' ${Object.keys( - history + sorted ).length.toLocaleString()} paths into ${outputFile}` ) );