search

hypothesis / product-backlog

Where new feature ideas and current bugs for the Hypothesis product live
119 stars 7 forks source link

OpenStax book embedded as a URL in the LMS app keeps Hypothesis embedded, with "LMS" in the URL, when links are clicked #1411

Open mkdir-washington-edu opened 1 year ago

mkdir-washington-edu commented 1 year ago

Bug report form

Steps to reproduce

  1. Create an assignment from the page https://openstax.org/books/university-physics-volume-1/pages/1-introduction (example here, login in 1pass: "Canvas: Professor D")
  2. Click a link on that page
  3. Link opens in a new tab with a URL like: https://lms-viahtml.hypothes.is/proxy/mp_/https://openstax.org/books/university-physics-volume-1/pages/1-1-the-scope-and-scale-of-physics
  4. App is embedded, Public group is visible.

Expected behaviour

The page should open in a new tab, but without "https://lms-viahtml.hypothes.is/proxy/mp_/" and without the app embedded or in via at all.

Browser/system information

Chrome 107 MacOS 11.6

Additional details

Old issue with OpenStax here: https://github.com/hypothesis/product-backlog/issues/1273. Recently closed as the old issue seems to no longer happen. May also be related to: https://github.com/hypothesis/viahtml/issues/423

robertknight commented 1 year ago

It looks like the logic we have for preventing the URLs of <a> links in the original page from being rewritten into proxy links (see https://github.com/hypothesis/viahtml/blob/bd5ecfa387ff84a0d2c093e37469dce3500d0bab/viahtml/hooks/hooks.py#L96) is not working in this case.

What is supposed to happen for <a> links on the page is:

  1. On the backend, we prevent these links from being rewritten to go through ViaHTML
  2. On the frontend, we add a JS "click" handler for the document that adds target=_blank to the link when it is clicked, so it opens in an external tab (see https://github.com/hypothesis/viahtml/blob/bd5ecfa387ff84a0d2c093e37469dce3500d0bab/viahtml/templates/banner.html#L90)