Skip to content

Commit

Permalink
Avoid placing hints a <sup> elements
Browse files Browse the repository at this point in the history
Closes #84. In that issue, it looked like some sentences got no hints.
In reality, they did, but they were placed at `<sup>` elements in the
sentences and ended up behind the hints for selecting only the `<sup>`
text.

This PR tries to detect potential `<sup>` elements and prefers the start
of the text instead in those cases.

Unfortunately it is very difficult to tell `<sup>` and wrapped text
apart, so this isn't perfect. But at least it's better. Probably good in
all realistic cases.
  • Loading branch information
lydell committed Aug 18, 2024
1 parent 0df3336 commit 14787ee
Show file tree
Hide file tree
Showing 2 changed files with 81 additions and 4 deletions.
53 changes: 53 additions & 0 deletions html/selectable/index.html
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,41 @@ <h1>Selectable elements</h1>

<p>Simple text</p>
<p>Simple text with <em>nested</em> element.</p>
<p>Simple text with <sup>sup</sup> element.</p>
<p>
Simple text with
<span style="vertical-align: super">fake sup</span> element.
</p>
<p>
Simple text with
<sup>sup<sup>sup</sup></sup>
element.
</p>
<p>
Simple text with
<sup
>sup<sup>sup<sup>sup</sup></sup></sup
>
element.
</p>
<p>
Simple text with
<sup
>sup<sup
>sup<sup>sup<sup>sup</sup></sup></sup
></sup
>
element. Unfortunately, the hint ends up on the top sup.
</p>
<p><sup>sup</sup> that starts the element.</p>
<p>
<a href="#">Link with <sup>sup</sup> element.</a>
</p>
<p>
And a <a href="#">Wrapped link with <sup>sup</sup><br />element.</a>
</p>
<p>Simple text with <sub>sub</sub> element.</p>
<p><sub>sub</sub> that starts the element.</p>
<p>&nbsp;Simple text surrounded by whitespace&nbsp;</p>

<div>
Expand Down Expand Up @@ -79,6 +114,23 @@ <h1>Selectable elements</h1>
quis ad primis feugiat suscipit, cursus erat class ridiculus torquent
eleifend accumsan, gravida dis mattis potenti facilisis eu.
</p>
<p style="width: 300px; line-height: 0.8">
Text with very small line-height, causing the lines to overlap. Lorem
ipsum dolor sit amet consectetur adipiscing elit nulla quis at iaculis, ac
facilisis natoque nibh nascetur potenti taciti integer.
</p>
<p style="width: 300px; line-height: 0.8; text-indent: 1em">
Indented text with very small line-height, causing the lines to overlap.
Unfortunately, the hint ends up on line two (the code thinks we have a
super-script). Lorem ipsum dolor sit amet consectetur adipiscing elit
nulla quis at iaculis, ac facilisis natoque nibh nascetur potenti taciti
integer.
</p>
<p style="width: 300px; text-indent: 1em">
Indented text with very regular line-height. Lorem ipsum dolor sit amet
consectetur adipiscing elit nulla quis at iaculis, ac facilisis natoque
nibh nascetur potenti taciti integer.
</p>

<p>&lt;img&gt;:</p>
<img src="image.svg" alt="" />
Expand Down Expand Up @@ -144,6 +196,7 @@ <h1>Selectable elements</h1>
</svg>

<div style="height: 100vh"></div>
<p>Simple text with <sup>sup</sup> element.</p>

<script>
for (const canvas of document.querySelectorAll("canvas")) {
Expand Down
32 changes: 28 additions & 4 deletions src/worker/ElementManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2086,14 +2086,38 @@ function getBestNonEmptyTextPoint({
// text text text [F]link
// link text text
//
// Unfortunately it is very difficult to tell apart wrapped lines and lines
// with `<sup>`, so we do some heuristics.
if (preferTextStart) {
// Prefer the top-most part of the line. In case of a tie, prefer the
// left-most one.
const leftMostRect = rects.reduce((a, b) =>
// We usually prefer the top line (for wrapped text). (If multiple rects are
// at the same y coordinate, choose the left-most.)
const topMostRect = rects.reduce((a, b) =>
b.top < a.top ? b : b.top === a.top && b.left < a.left ? b : a
);

// But that might be a `<sup>` – then we want don’t want the hint to be at
// the `<sup>`, it should be at the start of the text. So also look for the
// left-most rect. (Multiple lines might touch the same left-most
// x-coordinate; choose the top-most of those.)
const leftMostRect = rects.reduce((a, b) =>
b.left < a.left ? b : b.left === a.left && b.top < a.top ? b : a
);

const xDistance = topMostRect.left - leftMostRect.left;
const yDistance = leftMostRect.top - topMostRect.top;

const rectsOverlapVertically = leftMostRect.top < topMostRect.bottom;

// Prefer the top-most rect, but if they overlap vertically (which indicates
// we might have a `<sup>` (or `<sub>` at the start of a sentence)), prefer
// the one closest to the top-left corner.
const rect =
rectsOverlapVertically && xDistance > yDistance
? leftMostRect
: topMostRect;

return {
...getXY(leftMostRect),
...getXY(rect),
align,
debug: "getBestNonEmptyTextPoint preferTextStart",
};
Expand Down

0 comments on commit 14787ee

Please sign in to comment.