1. 26 Jul, 2021 1 commit
  2. 25 Jul, 2021 3 commits
    • Robert Knight's avatar
      Replace `sidebarWindow` property with `sidebar.iframe.contentWindow` · 4305f258
      Robert Knight authored
      Rather than use a dedicated getter to expose the sidebar frame, it
      seemed better on a second look to use the existing `sidebar.iframe`
      property that exposes the `<iframe>` element, but document and test that
      property.
      4305f258
    • Robert Knight's avatar
      Add additional comments and revise logic slightly in annotator/index.js · 36fe2c08
      Robert Knight authored
      Add comments to clarify various aspects of the way the host/guest frames
      are set up and interact.
      36fe2c08
    • Robert Knight's avatar
      Explicitly specify guest/sidebar frames to ping in `startDiscovery` · 3dd8144a
      Robert Knight authored
      The logic used by guest/host and sidebar frames to find each other in
      `Discovery#startDiscovery` relied on traversing the frame tree starting
      from `window.top` using `window.frames`. Since `window.frames` doesn't
      include frames in shadow roots, this did not work if _both_ the guest and
      sidebar are contained within a shadow root. The sidebar is always
      contained in a shadow root created by the `<hypothesis-sidebar>`
      element, so this simplifies to only working if the guest/host is not
      contained in a shadow root.
      
      For current use cases it is possible for guest frames to get a direct
      reference to the sidebar frame they should be communicating with and for
      the sidebar to get direct a reference to the host frame (its parent).
      This avoids the need for `window.frames` traversal.
      
      This commit leverages that by changing `startDiscovery` to take an
      explicit array of frames to ping. Guest frames invoke `startDiscovery`
      passing a reference to the sidebar frame and sidebar frames invoke
      `startDiscovery` passing a reference to the parent (host) frame.
      
      This fixes the following scenarios:
      
       - Hypothesis not completing loading if host frame is contained within a
         shadow root. eg. In the VitalSource book reader.
      
       - Same-origin guest frames loading Hypothesis after the sidebar has
         already loaded. See http://localhost:3000/document/parent-frame test
         case in client dev server.
      
       - Multiple sidebars attempting to connect to the same guest frame, if
         Hypothesis is loaded multiple times in different parts of the frame
         tree. See http://localhost:3000/document/multi-frames test case in
         client dev server.
      
      This approach has some limitations:
      
       - Child guest frames which are not same-origin with the parent host frame
         are not supported.
       - Child guest frames which attempt to connect to the sidebar before it
         has loaded will fail to connect. This could be remedied by making the
         guest wait for confirmation of the sidebar having loaded before
         calling `connectToSidebar`. This doesn't affect the host frame since
         the sidebar pings that frame directly.
      
      These limitations are acceptable in the short term but will be remedied
      by a larger overhaul of inter-frame communication that is currently
      being worked on.
      3dd8144a
  3. 22 Jul, 2021 2 commits
    • Robert Knight's avatar
      Add test case for loading Hypothesis into a Shadow DOM-ed iframe · 5ca9d398
      Robert Knight authored
      Add a test case that reproduces the scenario encountered in the new
      VitalSource book reader [1] where Hypothesis is loaded into an iframe
      whose `<iframe>` element is contained within a shadow root and is thus
      not accessible via `window.frames`. This scenario currently prevents the
      Hypothesis sidebar from discovering and connecting to the frame.
      5ca9d398
    • Robert Knight's avatar
      Split test documents into general and specific-scenario tests · fd5a1307
      Robert Knight authored
      Split the test HTML and PDF documents into two groups: those that test
      general usage of Hypothesis with a particular document type using the
      default configuration and those that test specific scenarios, edge cases
      or non-standard configuration.
      
      The link descriptions for the scenario tests have been changed to focus
      on the scenario, since that is what is important, rather than the
      document title.
      fd5a1307
  4. 21 Jul, 2021 6 commits
  5. 19 Jul, 2021 7 commits
  6. 16 Jul, 2021 7 commits
  7. 15 Jul, 2021 8 commits
  8. 14 Jul, 2021 6 commits
    • Robert Knight's avatar
      Use `$imports.$mock` to mock RPC in bridge-test.js · b5e717ae
      Robert Knight authored
      The `Bridge` code was written before we had good mocking tools in the
      client. As a result the `Bridge` class mocks `RPC` in an unconventional
      way where it constructs the real class, but then stubs out specific
      methods. Also tests varied in the degree to which they tested only
      `Bridge` vs Bridge + RPC together.
      
      This commit mocks `RPC` in bridge-test.js using our standard mocking
      tools and makes all tests in the file unit tests. This involved some
      significant changes to the `onConnect` tests.
      b5e717ae
    • Eduardo Sanz García's avatar
      Various improvements based on PR feedback · 12982d25
      Eduardo Sanz García authored
      * change `@deprecated` for a comment
      * added a test to cover some missing lines in `_isValidSender` function.
      * `_isValidMessage` renamed to `_parseMessage`
      * created helper function `waitForMessageDelivery`
      * parametrise test
      12982d25
    • Eduardo Sanz García's avatar
      Cleanup/follow up work on `RPC` · 1aea95e6
      Eduardo Sanz García authored
      RPC:
      * added `@deprecated` statements and additional comments
      * removed `this._destroyed` for the listener checks, because the
        listener is removed when called `destroy()`.
      * added `this._port.close()` in addition to removing the listner.
      
      `bridge-test`:
      * refactor all the `function () {}` to arrow functions
      
      `frame-rpc-test.js`:
      * added these tests to check the `MessageChannel` functionality.
      1aea95e6
    • Eduardo Sanz García's avatar
      Strengthen types · 3e94f694
      Eduardo Sanz García authored
      3e94f694
    • Eduardo Sanz García's avatar
      Simplify internals · b0f23457
      Eduardo Sanz García authored
      Created a couple of utility methods to avoid duplication of code and
      enable to decouple RPC for `Window` or for `MessagePort`.
      b0f23457
    • Eduardo Sanz García's avatar
      Add `ListenerCollection` utility to handle listeners · 75525988
      Eduardo Sanz García authored
      It simplifies the code.
      75525988