Skip to content

Commit

Permalink
fix: workaround Safari bug for bounding boxes at line breaks
Browse files Browse the repository at this point in the history
  • Loading branch information
birtles committed Apr 17, 2023
1 parent 34db842 commit f8fd8c5
Show file tree
Hide file tree
Showing 5 changed files with 52 additions and 12 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@ app.
- (Chrome, Edge, Safari) Fixed a bug where overlays would be broken on
[MangaDex](https://mangadex.org/)
([#1110](https://github.com/birchill/10ten-ja-reader/issues/1110)).
- (Safari) Improved popup placement when looking up words at the start of
a line.
- (Safari) Fixed a bug where the toolbar icon would get stuck not updating.

## [1.13.6] - 2023-02-23 (Firefox, Thunderbird only)
Expand Down
19 changes: 13 additions & 6 deletions src/content/get-cursor-position.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,10 @@
import { html } from '../utils/builder';
import { isTextInputNode, isTextNode, SVG_NS } from '../utils/dom-utils';
import { bboxIncludesPoint, Point } from '../utils/geometry';
import { getRangeForSingleCodepoint } from '../utils/range';
import {
getBboxForSingleCodepointRange,
getRangeForSingleCodepoint,
} from '../utils/range';
import { isChromium } from '../utils/ua-utils';

import { isGdocsOverlayElem } from './gdocs-canvas';
Expand Down Expand Up @@ -294,8 +297,9 @@ function getVisualOffset({
direction: 'backwards',
});

const previousCharacterBbox = range.getBoundingClientRect();
return bboxIncludesPoint({ bbox: previousCharacterBbox, point })
const previousCharacterBbox = getBboxForSingleCodepointRange(range);
return previousCharacterBbox &&
bboxIncludesPoint({ bbox: previousCharacterBbox, point })
? range.startOffset
: position.offset;
}
Expand Down Expand Up @@ -448,7 +452,10 @@ function getDistanceFromTextNode(

// Get bbox of first character in range (since that's where we select from).
const range = getRangeForSingleCodepoint({ source: node, offset });
const bbox = range.getBoundingClientRect();
const bbox = getBboxForSingleCodepointRange(range);
if (!bbox) {
return null;
}

// Find the distance from the cursor to the closest edge of that character
// since if we have a large font size the two distances could be quite
Expand Down Expand Up @@ -662,8 +669,8 @@ function adjustForRangeBoundary({
offset: 0,
});

const firstCharBbox = firstCharRange.getBoundingClientRect();
if (!bboxIncludesPoint({ bbox: firstCharBbox, point })) {
const firstCharBbox = getBboxForSingleCodepointRange(firstCharRange);
if (!firstCharBbox || !bboxIncludesPoint({ bbox: firstCharBbox, point })) {
return range;
}

Expand Down
13 changes: 9 additions & 4 deletions src/content/get-text.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
import { bboxIncludesPoint, Point } from '../utils/geometry';
import { getRangeForSingleCodepoint } from '../utils/range';
import {
getBboxForSingleCodepointRange,
getRangeForSingleCodepoint,
} from '../utils/range';

import { getContentType } from './content-type';
import { getTextFromAnnotatedCanvas } from './gdocs-canvas';
Expand Down Expand Up @@ -168,9 +171,11 @@ function getFirstCharBbox(position: CursorPosition): DOMRect | undefined {
});

// Skip empty ranges
return firstCharRange.startOffset !== firstCharRange.endOffset
? firstCharRange.getBoundingClientRect()
: undefined;
if (firstCharRange.collapsed) {
return undefined;
}

return getBboxForSingleCodepointRange(firstCharRange);
}

function getTextNodeStart({
Expand Down
13 changes: 11 additions & 2 deletions src/content/target-props.ts
Original file line number Diff line number Diff line change
Expand Up @@ -117,12 +117,21 @@ function getInitialClientBboxofTextSelection(
})[0];
} else if (range) {
range.setEnd(node, end);
result[size] = range.getClientRects()[0];

// Safari will sometimes return zero-width bboxes when the range starts
// on a new line so we should make sure to choose the wider bbox.
const bbox = [...range.getClientRects()].reduce<DOMRect | undefined>(
(result, bbox) =>
(result?.width || 0) >= bbox.width ? result : bbox,
undefined
);

// Sometimes getClientRects can return an empty array
if (!result[size]) {
if (!bbox) {
return undefined;
}

result[size] = bbox;
}

lastEnd = end;
Expand Down
17 changes: 17 additions & 0 deletions src/utils/range.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,3 +34,20 @@ export function getRangeForSingleCodepoint({

return range;
}

export function getBboxForSingleCodepointRange(
range: Range
): DOMRect | undefined {
// In Safari when a range is at the start of a line, getClientRects()
// returns two bounding boxes: an empty (zero-width) one at the end of the
// line and a non-empty one for the first character at the start of the line.
//
// Worse still, getBoundingClientRect() returns the union of the two producing
// a massive (and very wrong) bounding box.
//
// Here we get the individual client rects and then return the widest one.
return [...range.getClientRects()].reduce<DOMRect | undefined>(
(result, bbox) => ((result?.width || 0) >= bbox.width ? result : bbox),
undefined
);
}

0 comments on commit f8fd8c5

Please sign in to comment.