1. 20 Nov, 2020 11 commits
    • Robert Knight's avatar
    • Robert Knight's avatar
      Clarify what happens if range is outside of text layer · f6f0003f
      Robert Knight authored
      Ensure that the `describe` function in `anchoring/pdf.js` fails with a
      useful error if the passed range includes text outside of the PDF page's
      text layer. In this case `getNodeTextLayer` will return `null` and
      existing code in `describe` will throw to indicate the range starts or
      ends outside of the text layer.
      
      This will help to avoid the confusion that arose in
      https://github.com/hypothesis/client/issues/1464.
      
      This behavior is still not really useful from the end user's point of
      view, but at least the error is better. Improving the actual behavior
      will come later.
      f6f0003f
    • Robert Knight's avatar
      Remove unused dom-seek dependency · ef5f9e22
      Robert Knight authored
      ef5f9e22
    • Robert Knight's avatar
      Remove unused `toRange` implementation · 0f7ee503
      Robert Knight authored
      This has been replaced by the `TextRange.toRange` method in `text-range.js`.
      The test cases are covered by the tests for `TextRange` and
      `TextPosition`.
      0f7ee503
    • Robert Knight's avatar
      Convert PDF anchoring to use TextPosition / TextRange classes · e92b7fe9
      Robert Knight authored
      The PDF anchoring code was using several different dependencies to
      to handle converting between ranges and text positions, as well as
      normalizing ranges to that they start and end in text nodes.
      
      This commit simplifies the picture by converting the PDF anchoring code
      to use only the `TextRange` and `TextPosition` classes for these
      purposes.
      
       - Change the PDF anchoring code to use the `TextRange` and
         `TextPosition` classes to handle converting a DOM range into text
         offsets in `describe`
      
       - Change the PDF anchoring code to use `TextRange` to handle resolving
         a (start, end) pair of character offsets within a page to a `Range`
      
       - Improve the type information for several related functions within the
         PDF anchoring code
      e92b7fe9
    • Robert Knight's avatar
      Rewrite a comment · b87e1395
      Robert Knight authored
      Try to explain the issue with `textContent` in a better way.
      b87e1395
    • Robert Knight's avatar
      Make TextRange ignore text in comments and processing instructions · dd0af4d8
      Robert Knight authored
      `Node.textContent` has a quirk where it does not count text in comments
      or processing instructions when called on an `Element` but does return
      the comment / processing instruction data if called directly on a
      `Comment` or `ProcessingInstruction` node.
      
      To make conversion between `Range`s and `TextRange`s consistent in both
      directions we need to always ignore text in comments/processing
      instructions.
      dd0af4d8
    • Lyza Danger Gardner's avatar
      Add NOTEBOOK_APP_URL to env var docs · ba2d2c00
      Lyza Danger Gardner authored
      ba2d2c00
    • Lyza Danger Gardner's avatar
    • Robert Knight's avatar
      Add test to check handling of null selection · f208cf46
      Robert Knight authored
      There is a difference between an empty selection and no selection at
      all. In the latter case APIs such as `selection.getRangeAt(0)` will
      throw an error.
      f208cf46
    • Robert Knight's avatar
      Hide annotation layers in PDF when there is a selection · bb1dc45e
      Robert Knight authored
      Content in a PDF's annotation layer, such as link annotations, can
      interfere with text selection and also cause creating annotations to
      fail [1]. This commit resolves the problem by temporarily hiding annotation
      layers when there is a non-empty selection. This allows the user to
      select text inside link annotations (for example) while still enabling
      the user to activate the link normally when there is no selection.
      
      Fixes https://github.com/hypothesis/client/issues/1464
      
      [1] This is because the annotation layer is outside the text layer and
          the PDF anchoring code expects the selection to be within the text
          layer.
      bb1dc45e
  2. 18 Nov, 2020 2 commits
    • Robert Knight's avatar
      Optimize `TextRange.toRange` if both positions are in same element · e8ed0d57
      Robert Knight authored
      Optimize the case where `toRange` is used to resolve (start, end)
      offsets within the same element by only iterating over nodes in the
      element once, instead of iterating separately to resolve the start and
      end points.
      e8ed0d57
    • Robert Knight's avatar
      Add `TextPosition.relativeTo` method · e0592c09
      Robert Knight authored
      Add a method that converts a text position within an element to one
      where the offset is relative to some ancestor element.
      
      This operation will be needed for replacing existing Range => (element,
      start, end) conversions with a single one based on `TextRange` and
      `TextPosition`.
      e0592c09
  3. 16 Nov, 2020 15 commits
  4. 13 Nov, 2020 4 commits
  5. 12 Nov, 2020 4 commits
    • Robert Knight's avatar
      Improve some JSDoc comments and a variable name · fdc5a1bd
      Robert Knight authored
      Make some small improvements in response to PR feedback.
      fdc5a1bd
    • Robert Knight's avatar
      Add more tests for splitting text nodes and fix an edge case · 5f7be6a3
      Robert Knight authored
      Add additional tests for `highlightRange` calls that requires splitting
      text nodes in order to wrap only the part inside the range. This turned
      up an edge case with handling collapsed ranges that start (and end) in
      the middle of a text node.
      5f7be6a3
    • Robert Knight's avatar
      Make `highlightRange` accept DOM range objects · bb69d03e
      Robert Knight authored
      Decouple `highlightRange` from the `NormalizedRange` class and make it
      work with ordinary DOM ranges. This decouples the highlighter
      implementation from the range wrapper types, which will make some future changes
      to highlighting easier.
      bb69d03e
    • Robert Knight's avatar
      Optimize and fix a bug in `isNodeInRange` if node had no parent · 7d196444
      Robert Knight authored
      `isNodeInRange` in range would throw an exception if the passed node had
      no parent because `range.selectNode(...)` requires its argument to have
      a parent.
      
      This commit rewrites `isNodeInRange` to use `range.comparePoint` which
      avoids creating a temporary live range and handle exceptions that
      `comparePoint` may throw. It also updates the JSDoc to more accurately
      describe what the function does.
      7d196444
  6. 11 Nov, 2020 1 commit
  7. 10 Nov, 2020 2 commits
    • Robert Knight's avatar
      Don't try to resolve sidebar app URL when booting sidebar app · 9d0621df
      Robert Knight authored
      When attempting to create a build of the browser extension using the
      local dev client and the production h service I encountered an issue where
      resolving the `sidebarAppUrl` setting in `src/boot/index.js` failed.
      
      This happens because the extension does not set this setting in the
      sidebar app so the boot script fell back to trying to resolve the URL
      template baked into the development client. This in turn failed
      because `document.currentScript` is not set in this context.
      
      The `sidebarAppUrl` setting is not actually needed when booting the sidebar app, so
      this commit refactors the boot code to avoid generating it in this
      context. This is done by moving the responsibility for determining which
      part of the client to load from `boot/boot.js` into `boot/index.js` and
      only resolving the necessary settings.
      9d0621df
    • Eduardo Sanz García's avatar
      Remove npm depency · 0079b15d
      Eduardo Sanz García authored
      Rely exclusively on `yarn`.
      0079b15d
  8. 09 Nov, 2020 1 commit