Skip to content

Commit 59689fa

Browse files
robinmetralgijsk
andauthored
Handle en and em dashes in titles (mozilla#960)
* Handle en and em dashes in titles * Update remaining regexes * Add test case * Update existing test fixtures An existing test case had a title with an em dash. It started to fail since it wasn't previously detected as a separator. * Move title separators into a variable * Use \s instead of whitespace * Keep titleSeparators in a regex instead of a string This makes escaping easier to reason about, but has the downside of having to call .source each time we compose it. * Inline .source access when defining titleSeparators. --------- Co-authored-by: Gijs Kruitbosch <gijskruitbosch@gmail.com>
1 parent 2134a18 commit 59689fa

5 files changed

Lines changed: 57 additions & 6 deletions

File tree

Readability.js

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -593,14 +593,20 @@ Readability.prototype = {
593593
}
594594

595595
// If there's a separator in the title, first remove the final part
596-
if (/ [\|\-\\\/>»] /.test(curTitle)) {
597-
titleHadHierarchicalSeparators = / [\\\/>»] /.test(curTitle);
598-
let allSeparators = Array.from(origTitle.matchAll(/ [\|\-\\\/>»] /gi));
596+
const titleSeparators = /\|\-\\\/>»/.source;
597+
if (new RegExp(`\\s[${titleSeparators}]\\s`).test(curTitle)) {
598+
titleHadHierarchicalSeparators = /\s[\\\/>»]\s/.test(curTitle);
599+
let allSeparators = Array.from(
600+
origTitle.matchAll(new RegExp(`\\s[${titleSeparators}]\\s`, "gi"))
601+
);
599602
curTitle = origTitle.substring(0, allSeparators.pop().index);
600603

601604
// If the resulting title is too short, remove the first part instead:
602605
if (wordCount(curTitle) < 3) {
603-
curTitle = origTitle.replace(/^[^\|\-\\\/>»]*[\|\-\\\/>»]/gi, "");
606+
curTitle = origTitle.replace(
607+
new RegExp(`^[^${titleSeparators}]*[${titleSeparators}]`, "gi"),
608+
""
609+
);
604610
}
605611
} else if (curTitle.includes(": ")) {
606612
// Check if we have an heading containing this exact string, so we
@@ -642,7 +648,10 @@ Readability.prototype = {
642648
curTitleWordCount <= 4 &&
643649
(!titleHadHierarchicalSeparators ||
644650
curTitleWordCount !=
645-
wordCount(origTitle.replace(/[\|\-\\\/>»]+/g, "")) - 1)
651+
wordCount(
652+
origTitle.replace(new RegExp(`\\s[${titleSeparators}]\\s`, "g"), "")
653+
) -
654+
1)
646655
) {
647656
curTitle = origTitle;
648657
}

test/test-pages/mercurial/expected-metadata.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
{
2-
"title": "Shared Mutable History — evolve extension for Mercurial",
2+
"title": "Evolve: Shared Mutable History — evolve extension for Mercurial",
33
"byline": null,
44
"dir": null,
55
"excerpt": "Contents",
Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
{
2+
"title": "Lorem ipsum dolor sit amet",
3+
"byline": null,
4+
"dir": null,
5+
"excerpt": "Lorem ipsum dolor sit amet, consectetur adipisicing elit, sed do eiusmod\n incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam,\n quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo\n consequat. Duis aute irure dolor in reprehenderit in voluptate velit esse\n cillum dolore eu fugiat nulla pariatur. Excepteur sint occaecat cupidatat non\n proident, sunt in culpa qui officia deserunt mollit anim id est laborum.",
6+
"siteName": null,
7+
"publishedTime": null,
8+
"readerable": true
9+
}
Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
<div id="readability-page-1" class="page">
2+
<article>
3+
<p> Lorem ipsum dolor sit amet, consectetur adipisicing elit, sed do eiusmod incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat. Duis aute irure dolor in reprehenderit in voluptate velit esse cillum dolore eu fugiat nulla pariatur. Excepteur sint occaecat cupidatat non proident, sunt in culpa qui officia deserunt mollit anim id est laborum. </p>
4+
<p> Lorem ipsum dolor sit amet, consectetur adipisicing elit, sed do eiusmod incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat. Duis aute irure dolor in reprehenderit in voluptate velit esse cillum dolore eu fugiat nulla pariatur. Excepteur sint occaecat cupidatat non proident, sunt in culpa qui officia deserunt mollit anim id est laborum. </p>
5+
</article>
6+
</div>
Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
<!DOCTYPE html>
2+
<html>
3+
<head>
4+
<meta charset="utf-8"/>
5+
<title>Lorem ipsum dolor sit amet – My website</title>
6+
</head>
7+
<body>
8+
<article>
9+
<div>
10+
Lorem ipsum dolor sit amet, consectetur adipisicing elit, sed do eiusmod
11+
incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam,
12+
quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo
13+
consequat. Duis aute irure dolor in reprehenderit in voluptate velit esse
14+
cillum dolore eu fugiat nulla pariatur. Excepteur sint occaecat cupidatat non
15+
proident, sunt in culpa qui officia deserunt mollit anim id est laborum.
16+
</div>
17+
<div>
18+
Lorem ipsum dolor sit amet, consectetur adipisicing elit, sed do eiusmod
19+
incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam,
20+
quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo
21+
consequat. Duis aute irure dolor in reprehenderit in voluptate velit esse
22+
cillum dolore eu fugiat nulla pariatur. Excepteur sint occaecat cupidatat non
23+
proident, sunt in culpa qui officia deserunt mollit anim id est laborum.
24+
</div>
25+
</article>
26+
</body>
27+
</html>

0 commit comments

Comments
 (0)