1. 08 Feb, 2022 7 commits
    • Eduardo Sanz García's avatar
      Remove 'ready' RPC event · dc33e4be
      Eduardo Sanz García authored
      The new built-in "connect" PortRPC event (see #4175) makes the `ready`
      RPC event redundant. The host frame now uses the `connect` event to know
      when the sidebar frame is connected, and hence display it.
      dc33e4be
    • Robert Knight's avatar
      Make minor "close" event testing improvements · f6ca8fad
      Robert Knight authored
       - Use asserts to verify that "close" events are not emitted earlier
         than they should be, rather than blindly resetting the close event
         listener stub.
       - Reset the sinon sandbox after each test to ensure any DOM APIs are
         unmocked
      f6ca8fad
    • Robert Knight's avatar
      c23833a4
    • Robert Knight's avatar
      dd7d135d
    • Robert Knight's avatar
    • Robert Knight's avatar
      Add test that `close` is only invoked once · 512c9d2c
      Robert Knight authored
      512c9d2c
    • Robert Knight's avatar
      Add built-in "connect" and "close" events to PortRPC · 5ab98a3b
      Robert Knight authored
      Add two built-in events to PortRPC which are dispatched when PortRPC connects to
      a port and when it is destroyed or the containing frame is unloaded. These
      events can be used in the counterpart PortRPC to confirm that the sender has
      successfully received and connected to the port, and to get notified when the
      port goes away.
      
      Sending the "close" event in the context of a window unloading in Safari
      <= 15 requires a workaround that involves registering a handler in the
      parent frame. This handler is currently installed only in the host frame.
      
      The new "close" event is used to replace the "frameDestroyed" message
      that was used by guests to notify the sidebar when it went away. This solves
      several problems:
      
       - It centralizes the workaround for https://bugs.webkit.org/show_bug.cgi?id=231167
         in `post-rpc.js`, instead of having it spread between several
         modules.
      
       - It provides a way to tear down the guest-host connection when the
         guest goes away.
      
       - It provides a way to handle the case where a guest is unloaded before it
         has received and connected to the port, by having the host/sidebar frames
         expect the "connect" call within a timeout. This is not yet implemented.
      5ab98a3b
  2. 07 Feb, 2022 12 commits
  3. 04 Feb, 2022 4 commits
    • Eduardo Sanz García's avatar
      Show buckets in the bucket-bar for VitalSource · c3f6ebe9
      Eduardo Sanz García authored
      Background information
      
      Currently, the bucket-bar should display anchor positions only from a
      _single_ guest frame. This is because there is no merging mechanism for
      anchor positions from multiple guest frames.
      
      Before this PR
      
      * The host frame listened for `anchorsChanged` events from
      _all_ the guest frames.
      
      * Only _one_ guest frame sent this event. We referred to this guest
        frame as having the 'main' annotatable content.  The 'main'
        annotatable guest frame was identified by not having a
        `subFrameIdentifier` (a configuration option added to a guest frame
        when the Hypothesis client was injected). Hence, the 'main'
        annotatable guest frame was always the frame where the Hypothesis
        loaded initially (in contrast with the injection mechanism).
      
      In this PR
      
      We have reversed the logic of how the `anchorChanged` RPC
      events are send and received:
      
      * _Every_ guest frame sends `anchorsChanged` RPC events to the host
        frame.
      
      * The host frame chooses to listen to only _one_ guest frame for the
        `anchorsChanged` RPC events: the guest frame that connects first with
        the host.
      
      For non-VitalSource case, the first guest frame that connects to the
      host frame is the one where the Hypothesis client was initially loaded
      (doesn't contain `subFrameIdentifier` option).
      
      For the VitalSource, because of #4176, there is no guest frame in the
      host frame. This allows the guest frame in the book content, where the
      Hypothesis client was injected, to send anchor positions, and positions
      to be reflected in the host's bucket-bar.
      c3f6ebe9
    • Eduardo Sanz García's avatar
      Improve consistency on bucket hovering and focusing · 259d695e
      Eduardo Sanz García authored
      On #4069, we introduced a small improvement when hovering a left-pointed
      bucket: focus the corresponding annotation card, in addition to the
      anchor's highlight.
      
      In this PR, we introduce the same improvement to the up and down-pointed
      buckets.
      
      In addition, I have realised we handled `onBlur` but not `onFocus`
      events. I have added more complete support for keyboard navigation.
      
      I substituted `onMouseMove` for `onMouseEnter` because it is triggered
      less frequently.
      259d695e
    • Eduardo's avatar
      Apply suggestions from code review · 6ed8c93b
      Eduardo authored
      Co-authored-by: 's avatarRobert Knight <robertknight@gmail.com>
      6ed8c93b
    • Eduardo Sanz García's avatar
      Do not create a `Guest` instance in the VS container · 1b8e8a4d
      Eduardo Sanz García authored
      In order to not create an unnecessary `Guest` instance in the
      VitalSource container we needed to do the following:
      
      * The `VitalSourceContainerIntegration` is stripped down and converted
        to a class, `VitalSourceInjector`, that does not implement the
        Integration interface but is only responsible for injecting the client
        into content frames.
      
      * The host frame checks the VS frame role and either constructs a Guest
        or sets up the `VitalSourceInjector` as appropriate.
      
      * The `HypothesisInjector`-related logic in `Guest` is extracted out of
        that class and set up in the annotator entry point instead.
      1b8e8a4d
  4. 03 Feb, 2022 4 commits
  5. 02 Feb, 2022 8 commits
    • Robert Knight's avatar
      Revise comments for `frameForAnnotation` · 4c9bdda1
      Robert Knight authored
      4c9bdda1
    • Robert Knight's avatar
    • Robert Knight's avatar
    • Robert Knight's avatar
      Only send annotations to matching frame · 0a1077b3
      Robert Knight authored
      When the sidebar is connected to multiple guest frames it will send all
      incoming annotations to all frames. The result is typically that the
      annotation will anchor in one frame and orphan in the others. Depending
      on what order this happens in, the annotation will non-deterministically
      show up as an Annotation or Orphan in the sidebar.
      
      In order to determine which frames an annotation should be sent to in
      all cases, we'd either need the backend to return information about which
      search URIs an annotation matches or make a separate search request for
      each frame and record the associated frame with the results. This
      will require some significant refactoring of the annotation search
      service.
      
      As an interim step, make `FrameSyncService` send annotations only to a
      single frame based on matching URL, with a fallback to sending to the
      main frame if there is no exact match. This will work as expected for
      most pages, and is at least deterministic when it does fail. When we
      have a solution for being able to match annotations to frames more
      generally, we can adapt this code to use it.
      
      This is a partial solution to https://github.com/hypothesis/client/issues/3992.
      0a1077b3
    • Robert Knight's avatar
    • Robert Knight's avatar
    • Robert Knight's avatar
    • Robert Knight's avatar
      Revise how guest notifies host and sidebar when it is unloaded · 4b021476
      Robert Knight authored
      Change how the guest notifies other frames, specifically the sidebar and host,
      when it is unloaded. The host and sidebar now receive a `frameDestroyed` message
      from the corresponding guest port, which allows them to easily close the right
      port and remove it from the list of active guest ports.
      
      Due to a Safari bug (see code comments) we can't send the `frameDestroyed`
      messages from the guest frame while it is being unloaded. However it is possible
      to first transfer the port to the host frame and then have the host frame send
      the message on the same port. I also tried sending the message from the guest
      frame, and then transferring the port to the host frame but that didn't work.
      This workaround has the advantage that it is transparent to the receiver of the
      `frameDestroyed` message.
      
      This change is also a step towards possibly not relying on user-provided guest
      frame identifiers in the sidebar, which only become available once the
      `documentInfoChanged` call has been received. Instead the sidebar could
      use its own internal IDs for guest frames, avoiding the possibility for
      conflicts.
      
      Changes in detail:
      
       - Add `disconnect` method to PortRPC
      
       - When guest is unloaded, transfer ports to the host frame in a
         `hypothesisGuestUnloaded` message, and make the host frame dispatch
         `frameDestroyed` calls on these ports.
      
       - Handle `frameDestroyed` in sidebar by closing port, removing it from
         the active guest list and removing the associated frame from the
         store
      
       - Handle `frameDestroyed` in host frame by closing port and removing it
         from the active guest list
      4b021476
  6. 01 Feb, 2022 5 commits