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

generateFragmentFromRange from fragment-generation-utils.js is broken, infinite loop (Chrome, Safari, Firefox) #164

Open
danielweck opened this issue Oct 9, 2024 · 5 comments

Comments

@danielweck
Copy link

danielweck commented Oct 9, 2024

Tested with [email protected] and [email protected].

To reproduce:

  1. Load https://raw.githack.com/IDPF/epub3-samples/refs/heads/main/30/accessible_epub_3/EPUB/pr01.xhtml (I tested on MacOS with Firefox, Chrome and Safari)
  2. Open the developer console, and paste the Javascript code snippet below (note the 1-second debounce on the selectionchange event to avoid flooding generateFragmentFromRange() calls).
  3. Create a text selection in the webpage by double-clicking or dragging the cursor (I personally test selections in the first paragraph, so I can debug/trace character offsets in the search space(s) more easily).
  4. Observe in the console that the process is blocked, seems stuck in an infinite loop.
document.addEventListener("selectionchange", async () => {
    window._FRAG = window._FRAG || await import("https://unpkg.com/[email protected]/dist/fragment-generation-utils.js");
    if (window._selectionchangeTimeout) {
        clearTimeout(window._selectionchangeTimeout);
        window._selectionchangeTimeout = undefined;
    }
    const r = document.getSelection()?.getRangeAt(0);
    if (r && !r.collapsed) {
        console.log(">>> CHANGED document.getSelection().getRangeAt(0)");

        window._selectionchangeTimeout = setTimeout(() => {
            window._selectionchangeTimeout = undefined;

            console.log(">>>---- PRE generateFragmentFromRange...");
            const f = window._FRAG.generateFragmentFromRange(r, 1000);
            console.log(">>>---- POST generateFragmentFromRange:");
            console.log(JSON.stringify(f, null, 4));
        }, 1000);
    } else {
        console.log(">>> REMOVED / COLLAPSED document.getSelection().getRangeAt(0)");
    }
});

Unlike this original Javascript codebase, my TypeScript port runs but the TextFragment generation fails due to the "embiggen" logic not managing to create a unique TextFragment with prefix/suffix. I will report back here if / when I figure out what's going on :) (I "fixed" a couple of issues, notably in the text accumulator, perhaps that's why the original JS code loops indefinitely)

@tomayac
Copy link
Member

tomayac commented Oct 9, 2024

Thank you! Just released v6.0.0 with your recent fixes!

@danielweck
Copy link
Author

danielweck commented Oct 9, 2024

I tested on this page:

https://text-fragments-polyfill.glitch.me

=> status = 3 (TIMEOUT)

... so it's better than a seemingly infinite loop, but clearly something is wrong, somewhere :)
(I increased the timeout to 2s, just to be sure)

@danielweck
Copy link
Author

Thank you for releasing v6.0.0. The bug remains. I updated the above script to use the latest version 👍

@carlgieringer
Copy link

Hi, I am using text-fragments-polyfill in a Chrome extension that highlights webpages. I am encountering failures with 6.x that were not present in 5.7.6 that I think relate to this current issue.

E.g. if you:

  1. open this page
  2. enter the following script (this is the same script as above but I have removed the timeout: window._FRAG.setTimeout(null);)
  3. Select text from the "summary" section (see screenshot below.)

Then the outcomes differ between 5.7.6 and 6.x.

document.addEventListener("selectionchange", async () => {
    window._FRAG = window._FRAG || await import("https://unpkg.com/[email protected]/dist/fragment-generation-utils.js");
    window._FRAG.setTimeout(null);
    if (window._selectionchangeTimeout) {
        clearTimeout(window._selectionchangeTimeout);
        window._selectionchangeTimeout = undefined;
    }
    const r = document.getSelection()?.getRangeAt(0);
    if (r && !r.collapsed) {
        console.log(">>> CHANGED document.getSelection().getRangeAt(0)");

        window._selectionchangeTimeout = setTimeout(() => {
            window._selectionchangeTimeout = undefined;

            console.log(">>>---- PRE generateFragmentFromRange...");
            const f = window._FRAG.generateFragmentFromRange(r, 1000);
            console.log(">>>---- POST generateFragmentFromRange:");
            console.log(JSON.stringify(f, null, 4));
        }, 1000);
    } else {
        console.log(">>> REMOVED / COLLAPSED document.getSelection().getRangeAt(0)");
    }
});

6.0.0 and 6.1.0 both log AMBIGUOUS:

{
    "status": 2
}

whereas 5.7.6 logs:

{
    "status": 0,
    "fragment": {
        "textStart": "ranked-choice voting (rcv) is a confusing, chaotic “reform” being pushed by mega-liberal political donors and other activists."
    }
}
Screenshot 2024-10-17 at 8 53 10 AM

@carlgieringer
Copy link

I don't see a release/branch for 5.7.6, but based on the dates from NPM, I think this is the correct diff. I will start poking around to see if I have any idea what changed. :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants