hypothesis / product-backlog

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

VitalSource Assignments do not Display the Sidebar in Firefox #1549

Closed mkdir-washington-edu closed 1 month ago

mkdir-washington-edu commented 1 month ago

Bug report form

Steps to reproduce

  1. Open a Hypothesis assignment that uses a VitalSource text.
  2. Wait for the text to load, the text loads but the Sidebar doesn't

Expected behaviour

The VitalSource text should load along with the Hypothesis sidebar.

Actual behaviour

The VitalSource text loads without the Hypothesis sidebar.

image

Browser/system information

Firefox 128.0.3 (64-bit) MacOS Sonoma 14.5

Additional details

Assignment loads correctly on Google Chrome.

mkdir-washington-edu commented 1 month ago

@robertknight writes:

This looks like a problem with the VitalSource code that configures Hypothesis.

The exception being thrown is:

Uncaught DOMException: Permission denied to access property "document" on cross-origin object
    getAncestorLevel https://jigsaw.vitalsource.com/mosaic/wrapper.b40be880f2f52f8ee687.js:1
    _callee/_callee$/< https://jigsaw.vitalsource.com/mosaic/wrapper.b40be880f2f52f8ee687.js:1
    nrWrapper https://jigsaw.vitalsource.com/mosaic/wrapper.html?uuid=4b53a766-32c4-4ea1-d3f9-4e92f193075d&type=book:13
    triggerEvent https://jigsaw.vitalsource.com/mosaic/wrapper.b40be880f2f52f8ee687.js:1
    wrapTriggerEventWithMetadata https://jigsaw.vitalsource.com/mosaic/wrapper.b40be880f2f52f8ee687.js:1
    init https://jigsaw.vitalsource.com/mosaic/wrapper.b40be880f2f52f8ee687.js:1
    nrWrapper https://jigsaw.vitalsource.com/mosaic/wrapper.html?uuid=4b53a766-32c4-4ea1-d3f9-4e92f193075d&type=book:13

The getAncestorLevel function looks like:

    var ie = function getAncestorLevel() {
      if (void 0 !== location.ancestorOrigins) return location.ancestorOrigins.length;
      for (var ee = 0, te = window; te !== window.top; ) {
        if (te.document.referrer) try {
          ee++
        } catch (ee) {
        }
        te = te.parent
      }
      return ee
    }

This is then being called from this code:

             Object(oe.a) (
                              'window.hypothesisConfig = function () {return {'.concat(
                                ie() > 1 ? 'requestConfigFromFrame: {origin: "https://lms.hypothes.is", ancestorLevel: 3},' : '',
                                ' openSidebar: true };}'
                              ),
                              'hypotheconfig'
                            );

In the first block of code, note that Location.ancestorOrigins is a property that exists in Safari and Chrome but not Firefox. In the Firefox fallback path there is a mistake that the try...catch block is inside the if statement instead of containing the if statement. As a result if te.document.referrer throws, as it will do for a cross-origin window, the Hypothesis client is not loaded.

blakeperdue commented 1 month ago

@mkdir-washington-edu I'm a Product Director at VitalSource. We're looking into this issue and will report back when we know more and have updates. Thank you for reporting this!

blakeperdue commented 1 month ago

@mkdir-washington-edu We released a fix for this, please try it out and let us know if you still see any issues! Thank you!

mkdir-washington-edu commented 1 month ago

Thanks @blakeperdue. I'm seeing two issues. One, I'm being asked to log back into the VitalSource reader, which is not the behavior I'm used to after I've done so once and accepted cookies. Two, once the document loads in the reader, I think I'm missing the LMS config as the Hypothesis web app ends up loading.

I'll see about getting you more technical details.

robertknight commented 1 month ago

Hi @blakeperdue,

I can see there has been a code change, but unfortunately the new version still doesn't work the same in Firefox as it does in Chrome.

The technical details:

The new version of the compiled getAncestorLevel function mentioned in my earlier comment looks like:

  var ie = function getAncestorLevel() {
    if (void 0 !== location.ancestorOrigins) return location.ancestorOrigins.length;
    for (var ee = 0, te = window; te !== window.top; ) {
      try {
        te.document.referrer &&
        ee++
      } catch (ee) {
      }
      te = te.parent
    }
    return ee
  },

If if block is used by Chrome/Safari, which supports location.ancestorOrigins and the for loop is used by Firefox, which doesn't. If the document.referrer property access throws, as is the case when document is cross-origin, then the ee++ value stops getting incremented. As a result, the return value in Firefox doesn't match the return value in Chrome. As a result, the caller sets window.hypothesisConfig to a different value and VitalSource in our LMS app is not configured correctly.

mkdir-washington-edu commented 1 month ago

I see the Hypothesis Sidebar appearing over the VitalSource assignment in our demo environment. Thanks @blakeperdue!