swift-toolkit icon indicating copy to clipboard operation
swift-toolkit copied to clipboard

Decorations are incorrectly applied to paragraphs that start with a special character

Open streamg opened this issue 2 years ago • 4 comments

Bug Report

When applying decorations to ranges that are within a paragraph that starts with a character that has a different font, the decorations are shifted with one character.

What happened?

Taking the first paragraph from a epub source file:

<p class="x03-CO-Body-Text">Everyone knows that Harry began his career in One Direction&#x00A0;.&#x00A0;.&#x00A0;. or did he?! The name of his first band is super ironic considering the charmed career of charming Harry Styles—because in addition to skyrocketing, his career has gone in many directions!</p>

You can notice that the letter E is formatted differently. Because of that when applying a decoration on any of the words in this paragraph the decoration will be shifted with one character. IMG_A2656292FF4C-1

Example of a decoration described by locator:

{\"href\":\"/OEBPS/xhtml/05_Chapter_1_Styles_in_E.xhtml\",\"locations\":{\"progression\":0.062947067238912732,\"totalProgression\":0.2135789222699094},\"text\":{\"after\":\" his career in One Direction . . . or did he?! The name\",\"before\":\"STYLES IN EVERY DIRECTION\\n\\t\\t\\tEveryone knows that Harry \",\"highlight\":\"began\"},\"title\":\"Chapter 1: Styles in Every Direction\",\"type\":\"application/xhtml+xml\"}

Another issue us if we select a word from that paragraph. The selection Locator will describe the text but the after field has an additional character (the last character from the highlight). E.g. selecting the word "Harry" will return this Locator for the selection:

{\"href\":\"/OEBPS/xhtml/05_Chapter_1_Styles_in_E.xhtml\",\"locations\":{\"position\":6,\"progression\":0,\"totalProgression\":0.20833333333333334},\"text\":{\"after\":\"y began his career in One Direction . . . or did he?! The name of his first band is super ironic considering the charmed career of charming Harry Styles—because in addition to skyrocketing, his\",\"before\":\"CHAPTER 1\\n\\t\\t\\tSTYLES IN EVERY DIRECTION\\n\\t\\t\\tEveryone knows that\",\"highlight\":\"Harry\"},\"title\":\"Chapter 1: Styles in Every Direction\",\"type\":\"application/xhtml+xml\"}

Please find a sample epub here: 9780593383865_ewyrds_preview.epub.zip

The chapter name is 05_Chapter_1_Styles_in_E.xhtml

Expected behavior

The decorations should be properly applied and the current selection should return the correct locator.

How to reproduce?

Apply decorations for words in the mentioned paragraph and read the currentSelection.locator by selecting a word in the same paragraph.

<p class="x03-CO-Body-Text">Everyone knows that Harry began his career in One Direction&#x00A0;.&#x00A0;.&#x00A0;. or did he?! The name of his first band is super ironic considering the charmed career of charming Harry Styles—because in addition to skyrocketing, his career has gone in many directions!</p>

Environment

  • Readium version: 2.6.0

Development environment

macOS: 13.5.1 platform: arm64 carthage: Xcode 14.3 Build version 14E222b

Testing device

iOS version: 16.6.1 (not OS related) Model (e.g. iPhone 11 Pro Max): iPhone 12 Is it an emulator? No

Additional context

  • Are you willing to fix the problem and contribute a pull request? Yes, with proper guidance.

streamg avatar Nov 21 '23 11:11 streamg

Are you able to reproduce this issue with a highlight in the stock Test App? I'll reopen the issue if that's the case.

I believe the issue is caused by the strategy you used to produce the DOM range from the locator in your caching PR: https://github.com/readium/swift-toolkit/pull/354 I'll answer with more details on the PR.

mickael-menu avatar Nov 24 '23 16:11 mickael-menu

I'm reopening, I tested myself and could notice the same problem with a highlight.

mickael-menu avatar Nov 24 '23 16:11 mickael-menu

It looks like an issue in WebKit when using ::first-letter and getBoundingClientRects() which is used to layout the decorations.

You can reproduce it from the Safari Web Inspector:

  1. Select a portion of text.
  2. Get the range: const r = window.getSelection().getRangeAt(0)
  3. Observe the client rect: r.getBoundingClientRect() -> DOMRect {x: 197, y: 247, ...}
  4. Enable a property of the CSS pseudo-element ::first-letter on this paragraph. You can change only the text color which should not impact the position at all.
  5. Observe the client rect again, note that the x is shifted for some reason: r.getBoundingClientRect() -> DOMRect {x: 212, y: 247, ...}.

I don't think we can address this in the toolkit without removing the ::first-letter pseudo-elements.

mickael-menu avatar Nov 24 '23 17:11 mickael-menu

By the way it doesn't happen on your other sample because it doesn't use ::first-letter but a <span> element. In case you have control over the authoring...

Screenshot 2023-11-24 at 18 13 49

mickael-menu avatar Nov 24 '23 17:11 mickael-menu