Skip to content

Commit

Permalink
fix(flaws): don't report link to missing translation as broken if en-…
Browse files Browse the repository at this point in the history
…US fallback exists (#9408)

Co-authored-by: Claas Augner <[email protected]>
  • Loading branch information
yin1999 and caugner authored Mar 15, 2024
1 parent 24a2bf9 commit f9756d1
Show file tree
Hide file tree
Showing 2 changed files with 36 additions and 20 deletions.
40 changes: 26 additions & 14 deletions build/flaws/broken-links.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,13 +49,16 @@ function mutateLink(
) {
if (isSelfLink) {
$element.attr("aria-current", "page");
} else if (suggestion) {
$element.attr("href", suggestion);
} else if (enUSFallback) {
// If we have an English (US) fallback, then we use this first.
// As we still suggest the translated version even if we only
// have an English (US) version.
$element.attr("href", enUSFallback);
$element.append(` <small>(${DEFAULT_LOCALE})<small>`);
$element.addClass("only-in-en-us");
$element.attr("title", "Currently only available in English (US)");
} else if (suggestion) {
$element.attr("href", suggestion);
} else {
$element.addClass("page-not-created");
$element.attr("title", "This is a link to an unwritten page");
Expand Down Expand Up @@ -299,7 +302,6 @@ export function getBrokenLinksFlaws(
resolved + absoluteURL.search + absoluteURL.hash.toLowerCase()
);
} else {
let enUSFallbackURL = null;
// Test if the document is a translated document and the link isn't
// to an en-US URL. We know the link is broken (in this locale!)
// but it might be "salvageable" if we link the en-US equivalent.
Expand All @@ -316,28 +318,38 @@ export function getBrokenLinksFlaws(
`/${DEFAULT_LOCALE}/`
);
const enUSFound = Document.findByURL(enUSHrefNormalized);
// Note, we still recommend that contributors use localized links,
// even if the target document is still not localized.
if (enUSFound) {
enUSFallbackURL = enUSFound.url;
// Found the en-US version of the document. Just link to that.
mutateLink(a, null, enUSFound.url);
} else {
const enUSResolved = Redirect.resolve(enUSHrefNormalized);
let suggestion = null;
let enUSFallbackURL = null;
if (enUSResolved !== enUSHrefNormalized) {
enUSFallbackURL =
enUSResolved +
absoluteURL.search +
absoluteURL.hash.toLowerCase();
suggestion = enUSFallbackURL.replace(
`/${DEFAULT_LOCALE}/`,
`/${doc.locale}/`
);
}
addBrokenLink(
a,
checked.get(href),
href,
suggestion,
null,
enUSFallbackURL
);
}
} else {
// The link is broken and we don't have a suggestion.
addBrokenLink(a, checked.get(href), href);
}
addBrokenLink(
a,
checked.get(href),
href,
null,
enUSFallbackURL
? "Can use the English (en-US) link as a fallback"
: null,
enUSFallbackURL
);
}
}
// But does it have the correct case?!
Expand Down
16 changes: 10 additions & 6 deletions testing/tests/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1613,19 +1613,23 @@ test("translated content broken links can fall back to en-us", () => {
const { doc } = JSON.parse(fs.readFileSync(jsonFile, "utf-8")) as {
doc: Doc;
};

const map = new Map(doc.flaws.broken_links.map((x) => [x.href, x]));
expect(map.get("/fr/docs/Web/CSS/dumber").explanation).toBe(
"Can use the English (en-US) link as a fallback"
);
expect(map.get("/fr/docs/Web/CSS/number").explanation).toBe(
"Can use the English (en-US) link as a fallback"
);
expect(map.get("/fr/docs/Web/CSS/dumber")).toMatchObject({
explanation: "Can't resolve /fr/docs/Web/CSS/dumber",
suggestion: "/fr/docs/Web/CSS/number",
fixable: true,
line: 19,
column: 16,
});
expect(map.get("/fr/docs/Web/CSS/number")).toBeUndefined();

const htmlFile = path.join(builtFolder, "index.html");
const html = fs.readFileSync(htmlFile, "utf-8");
const $ = cheerio.load(html);
expect($('article a[href="/fr/docs/Web/CSS/dumber"]')).toHaveLength(0);
expect($('article a[href="/fr/docs/Web/CSS/number"]')).toHaveLength(0);
// Localized docs don't exist, but fallback to en-US is possible
expect($('article a[href="/en-US/docs/Web/CSS/number"]')).toHaveLength(2);
expect($("article a.only-in-en-us")).toHaveLength(2);
expect($("article a.only-in-en-us").attr("title")).toBe(
Expand Down

0 comments on commit f9756d1

Please sign in to comment.