Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
streamg opened this issue Nov 21, 2023 · 4 comments
Labels
bug Something isn't working

Comments

@streamg
Copy link

streamg commented Nov 21, 2023

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.
@mickael-menu
Copy link
Member

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: #354 I'll answer with more details on the PR.

@mickael-menu mickael-menu closed this as not planned Won't fix, can't repro, duplicate, stale Nov 24, 2023
@mickael-menu
Copy link
Member

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

@mickael-menu mickael-menu reopened this Nov 24, 2023
@mickael-menu mickael-menu added the bug Something isn't working label Nov 24, 2023
@mickael-menu
Copy link
Member

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
Copy link
Member

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants