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

Client not fully being "destroyed" with destroy event dispatched (unload-client.js) #3131

Closed
matt-e-king opened this issue Mar 10, 2021 · 6 comments

Comments

@matt-e-king
Copy link

matt-e-king commented Mar 10, 2021

I am using the Hypothesis Client embedded within a single page application (VueJS) with client side routing (Browser History API). I've been informed in previous discussions that Hypothesis doesn't directly support SPA implementation because of the dynamic nature of client side routing. The annotation selection targeting gets all screwy :)

However, @robertknight has recommended I use the browser-extension unload-client approach to reload the embedded client on demand. So in my VueJS application, when a route changes I run the code found in the unload-client script. The addEventListener for the destroy event can be found here.

This was working beautifully! However, as of late this appears to be broken - when I navigate pages the selection highlights become "stacked" on top of each other, the annotation counts double, and the Hypothesis menu when making a selection no longer works.

UPDATE: This bug however will most likely appear once the browser extension updates to the newest build. See the first set of steps to reproduce.

Steps to reproduce

If you have a local copy of the client and h running:

  1. Navigate to localhost:3000
  2. Click on "Open sidebar"
  3. Click on the "Page Note" icon in the Hypothesis sidebar, this should open up a new annotation input
  4. Click "Unload Client"
  5. Click "Load Client"
  6. You can no longer make a Page Note annotation or a text selection annotation. Here is a GIF (it loops so it might be confusing)
    HypothesisClientBug

Another way to recreate this "unload client" bug. You don't need to spin up an SPA or have Hypothesis locally to recreate this issue, the following steps should suffice:

  1. Turn off Hypothesis browser extension first - this seems to only be happening when embedding the client
  2. Navigate to https://web.hypothes.is/
  3. Open up the dev console and paste in the following to embed the client:
var hypothScript = document.createElement('script')
hypothScript.setAttribute('src', 'https://hypothes.is/embed.js')
hypothScript.setAttribute('async', true)
document.head.appendChild(hypothScript)
  1. This creates the Hypothesis sidebar on the right (make sure your annotation group is set to Public)
  2. Now paste the following code in the dev console:
var annotatorLink = document.querySelector(
    'link[type="application/annotator+html"]'
  );

  if (annotatorLink) {
    // Dispatch a 'destroy' event which is handled by the code in
    // annotator/main.js to remove the client.
    const destroyEvent = new Event('destroy');
    annotatorLink.dispatchEvent(destroyEvent);
  }

This code will dispatch a "destroy" event to the annotator client.

  1. The Hypothesis annotation sidebar should disappear
  2. Now paste the first code snippet again (from step 2), to "re-embed/reboot" the Hypothesis client
  3. Feel free to keep repeating steps 2 - 7
  4. All appears fine at first, but if you dev inspect one of the existing selections or look at one of the floating annotations counter affixed to the sidebar, the annotations are doubling each time you run this process. If you try and make another selection and click "Annotate" on the fly out menu, it doesn't work.
  5. All appears fine at first, but If you try and make a text selection and click "Annotate" on the fly out menu, the sidebar opens but does not provide a new annotation input box. you can't create a new Page Note either.

Expected behaviour

For the embedded client to full reboot and not leave any remnants behind.

Actual behaviour

See "Steps to reproduce"

Browser/system information

Latest version of Chrome

@matt-e-king
Copy link
Author

matt-e-king commented Mar 10, 2021

TL;DL; loading in an older version of the client appears to resolve my navigation issue.

I noticed my browser extension is version Version 1.690.0.0 (Official Build) and the embed script is pulling in 1.700.0.

So out of curiosity, I added:

<script type="application/json" class="js-hypothesis-config">
  {
    "assetRoot": "https://cdn.hypothes.is/hypothesis/1.600.0/"
  }
</script>

Which loaded an older version of the boot and bundle files (v1.600 just an arbitrary pick), and this more-or-less fixed the aforementioned "destroy" client bug. However, clicking selections no longer opens the sidebar - I'm guessing this isn't a "preferred" way to load a specific version. For instance the following are still being created from the embed.js script:

<link rel="sidebar" href="https://hypothes.is/app.html" type="application/annotator+html" data-hypothesis-asset>
<link rel="notebook" href="https://hypothes.is/notebook" type="application/annotator+html" data-hypothesis-asset>

And data-hypothesis-assetattr was added until after v1.600

Is there a preferred way to load an older version of the client?

@matt-e-king
Copy link
Author

Is it fair to say that when the browser extension updates to 1.700 a similar problem will occur if the user toggles the extension on and off?

@matt-e-king
Copy link
Author

UPDATE Steps to reproduce has been update due to some fixes that came with recent nightly builds of the Hypothesis client.

See Step #10 above.

@matt-e-king
Copy link
Author

UPDATE added new set of steps to reproduce once I got the local version of client and h up and running.

@esanzgar
Copy link
Contributor

esanzgar commented Mar 22, 2021

Does #3174 close this issue?

@matt-e-king
Copy link
Author

@esanzgar based on the testing I did locally, yes I believe so. Thanks!

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

2 participants