From 425b6e7787643f412105a2d7b8e09ffe00ba6481 Mon Sep 17 00:00:00 2001
From: Robert Knight <robertknight@gmail.com>
Date: Thu, 14 Oct 2021 09:23:26 +0100
Subject: [PATCH 1/4] Remove unused `visibleHighlights` state in store

There was an action to set this state but it was never read anywhere.
---
 src/sidebar/store/modules/test/viewer-test.js | 12 ------------
 src/sidebar/store/modules/viewer.js           | 13 -------------
 src/sidebar/store/test/index-test.js          |  9 ---------
 3 files changed, 34 deletions(-)

diff --git a/src/sidebar/store/modules/test/viewer-test.js b/src/sidebar/store/modules/test/viewer-test.js
index 6d454283b61..9b6bca93163 100644
--- a/src/sidebar/store/modules/test/viewer-test.js
+++ b/src/sidebar/store/modules/test/viewer-test.js
@@ -8,18 +8,6 @@ describe('store/modules/viewer', () => {
     store = createStore([viewer]);
   });
 
-  describe('#setShowHighlights', () => {
-    it('sets a flag indicating that highlights are visible', () => {
-      store.setShowHighlights(true);
-      assert.isTrue(store.getState().viewer.visibleHighlights);
-    });
-
-    it('sets a flag indicating that highlights are not visible', () => {
-      store.setShowHighlights(false);
-      assert.isFalse(store.getState().viewer.visibleHighlights);
-    });
-  });
-
   describe('hasSidebarOpened', () => {
     it('is `false` if sidebar has never been opened', () => {
       assert.isFalse(store.hasSidebarOpened());
diff --git a/src/sidebar/store/modules/viewer.js b/src/sidebar/store/modules/viewer.js
index 5bae7431ef7..3e051c24815 100644
--- a/src/sidebar/store/modules/viewer.js
+++ b/src/sidebar/store/modules/viewer.js
@@ -13,13 +13,9 @@ const initialState = {
    * current state of the sidebar, but tracks whether it has ever been open
    */
   sidebarHasOpened: false,
-  visibleHighlights: false,
 };
 
 const reducers = {
-  SET_HIGHLIGHTS_VISIBLE: function (state, action) {
-    return { visibleHighlights: action.visible };
-  },
   SET_SIDEBAR_OPENED: (state, action) => {
     if (action.opened === true) {
       // If the sidebar is open, track that it has ever been opened
@@ -34,14 +30,6 @@ const actions = util.actionTypes(reducers);
 
 // Action creators
 
-/**
- * Sets whether annotation highlights in connected documents are shown
- * or not.
- */
-function setShowHighlights(show) {
-  return { type: actions.SET_HIGHLIGHTS_VISIBLE, visible: show };
-}
-
 /**
  * @param {boolean} opened - If the sidebar is open
  */
@@ -59,7 +47,6 @@ export default createStoreModule(initialState, {
   namespace: 'viewer',
   reducers,
   actionCreators: {
-    setShowHighlights,
     setSidebarOpened,
   },
   selectors: {
diff --git a/src/sidebar/store/test/index-test.js b/src/sidebar/store/test/index-test.js
index 03a851f53d0..97815eb3a64 100644
--- a/src/sidebar/store/test/index-test.js
+++ b/src/sidebar/store/test/index-test.js
@@ -139,15 +139,6 @@ describe('createSidebarStore', () => {
     });
   });
 
-  describe('#setShowHighlights()', () => {
-    [{ state: true }, { state: false }].forEach(testCase => {
-      it(`sets the visibleHighlights state flag to ${testCase.state}`, () => {
-        store.setShowHighlights(testCase.state);
-        assert.equal(store.getState().viewer.visibleHighlights, testCase.state);
-      });
-    });
-  });
-
   describe('#updatingAnchorStatus', () => {
     it("updates the annotation's orphan flag", () => {
       const annot = defaultAnnotation();

From e3b77288d69b6bd241e6320d6d4bf96489792540 Mon Sep 17 00:00:00 2001
From: Robert Knight <robertknight@gmail.com>
Date: Thu, 14 Oct 2021 09:24:48 +0100
Subject: [PATCH 2/4] Make highlight visibility setting work in a multi-guest
 world

Make the client's highlight visibility state work in a sane way in
scenarios where there are multiple guests, or the main guest frame is not the
host frame. Previously there were several problems in these scenarios:

 - It was unclear which component of the application "owned" the
   highlight visibility state. The state in the sidebar could be changed
   as a result of the `highlightsVisibleChanged` event emitted by a
   Guest, as well as the user toggling highlight controls in the
   sidebar.

 - In the sidebar's `open` and `close` methods it directly set the
   highlight visibility in the main guest if the `showHighlights`
   setting was set to `whenSidebarOpen`. This meant that it didn't work
   for guests that were not in the main frame.

 - Guest frames could be configured with different `showHighlights`
   settings. In this case it was unclear what should happen.

This commit resolves this by making the `Sidebar` class in the host
frame the central owner of this state. It handles configuring the
initial state based on the `showHighlights` configuration setting, and
reflecting this state to the sidebar application which in turn reflects
it to guest frames. The initial visibility of highlights in a guest
frame is synchronized with this state when the guest frame connects to
the sidebar. This state is updated by the `Sidebar` class when:

 - The user toggles the highlight visibility control in the sidebar
 - A new highlight or annotation is created in a guest frame
 - The sidebar opens and closes, if the `showHighlights` configuration
   was set to `whenSidebarOpen`

Additionally the inconsistency of `setHighlightsVisible` vs
`setVisibleHighlights` in identifier and event names has been resolved by
using `setHighlightsVisible`/`highlightsVisible` everywhere.

Part of https://github.com/hypothesis/client/issues/3798
---
 src/annotator/config/index.js                |  2 +-
 src/annotator/config/test/index-test.js      |  4 +-
 src/annotator/guest.js                       | 25 +++++-------
 src/annotator/sidebar.js                     | 26 ++++++++-----
 src/annotator/test/guest-test.js             | 11 +++---
 src/annotator/test/sidebar-test.js           | 41 ++++++++++++++------
 src/sidebar/services/frame-sync.js           | 11 +++++-
 src/sidebar/services/test/frame-sync-test.js | 37 +++++++++++++-----
 8 files changed, 99 insertions(+), 58 deletions(-)

diff --git a/src/annotator/config/index.js b/src/annotator/config/index.js
index ae6179e49a8..f1c2917dc29 100644
--- a/src/annotator/config/index.js
+++ b/src/annotator/config/index.js
@@ -29,7 +29,7 @@ import { urlFromLinkTag } from './url-from-link-tag';
  */
 function configurationKeys(appContext) {
   const contexts = {
-    annotator: ['clientUrl', 'showHighlights', 'subFrameIdentifier'],
+    annotator: ['clientUrl', 'subFrameIdentifier'],
     sidebar: [
       'appType',
       'annotations',
diff --git a/src/annotator/config/test/index-test.js b/src/annotator/config/test/index-test.js
index 718f8c1c104..800eda31d24 100644
--- a/src/annotator/config/test/index-test.js
+++ b/src/annotator/config/test/index-test.js
@@ -216,7 +216,7 @@ describe('annotator/config/index', () => {
     [
       {
         app: 'annotator',
-        expectedKeys: ['clientUrl', 'showHighlights', 'subFrameIdentifier'],
+        expectedKeys: ['clientUrl', 'subFrameIdentifier'],
       },
       {
         app: 'sidebar',
@@ -252,7 +252,7 @@ describe('annotator/config/index', () => {
         ],
       },
     ].forEach(test => {
-      it(`ignore values not belonging to "${test.app}" context`, () => {
+      it(`ignores values not belonging to "${test.app}" context`, () => {
         const config = getConfig(test.app, 'WINDOW');
         assert.deepEqual(Object.keys(config), test.expectedKeys);
       });
diff --git a/src/annotator/guest.js b/src/annotator/guest.js
index 36c08d1dda0..69242148c68 100644
--- a/src/annotator/guest.js
+++ b/src/annotator/guest.js
@@ -124,7 +124,7 @@ export default class Guest {
   constructor(element, eventBus, config = {}, hostFrame = window) {
     this.element = element;
     this._emitter = eventBus.createEmitter();
-    this._visibleHighlights = false;
+    this._highlightsVisible = false;
     this._isAdderVisible = false;
 
     this._adder = new Adder(this.element, {
@@ -133,7 +133,6 @@ export default class Guest {
         /** @type {Selection} */ (document.getSelection()).removeAllRanges();
       },
       onHighlight: async () => {
-        this.setVisibleHighlights(true);
         await this.createAnnotation({ highlight: true });
         /** @type {Selection} */ (document.getSelection()).removeAllRanges();
       },
@@ -171,9 +170,6 @@ export default class Guest {
      * @type {Bridge<GuestToSidebarEvent,SidebarToGuestEvent>}
      */
     this._bridge = new Bridge();
-    this._bridge.onConnect(() => {
-      this.setVisibleHighlights(config.showHighlights === 'always');
-    });
     this._connectSidebarEvents();
 
     // Set up listeners for when the sidebar asks us to add or remove annotations
@@ -232,7 +228,7 @@ export default class Guest {
     this._listeners.add(this.element, 'mouseup', event => {
       const { target, metaKey, ctrlKey } = /** @type {MouseEvent} */ (event);
       const annotations = annotationsAt(/** @type {Element} */ (target));
-      if (annotations.length && this._visibleHighlights) {
+      if (annotations.length && this._highlightsVisible) {
         const toggle = metaKey || ctrlKey;
         this.selectAnnotations(annotations, toggle);
       }
@@ -250,13 +246,13 @@ export default class Guest {
 
     this._listeners.add(this.element, 'mouseover', ({ target }) => {
       const annotations = annotationsAt(/** @type {Element} */ (target));
-      if (annotations.length && this._visibleHighlights) {
+      if (annotations.length && this._highlightsVisible) {
         this._focusAnnotations(annotations);
       }
     });
 
     this._listeners.add(this.element, 'mouseout', () => {
-      if (this._visibleHighlights) {
+      if (this._highlightsVisible) {
         this._focusAnnotations([]);
       }
     });
@@ -360,8 +356,8 @@ export default class Guest {
     });
 
     // Handler for controls on the sidebar
-    this._bridge.on('setVisibleHighlights', showHighlights => {
-      this.setVisibleHighlights(showHighlights);
+    this._bridge.on('setHighlightsVisible', showHighlights => {
+      this.setHighlightsVisible(showHighlights);
     });
   }
 
@@ -663,12 +659,11 @@ export default class Guest {
   /**
    * Set whether highlights are visible in the document or not.
    *
-   * @param {boolean} shouldShowHighlights
+   * @param {boolean} visible
    */
-  setVisibleHighlights(shouldShowHighlights) {
-    setHighlightsVisible(this.element, shouldShowHighlights);
-    this._visibleHighlights = shouldShowHighlights;
-    this._emitter.publish('highlightsVisibleChanged', shouldShowHighlights);
+  setHighlightsVisible(visible) {
+    setHighlightsVisible(this.element, visible);
+    this._highlightsVisible = visible;
   }
 
   /**
diff --git a/src/annotator/sidebar.js b/src/annotator/sidebar.js
index 763b58d00de..e24413ba199 100644
--- a/src/annotator/sidebar.js
+++ b/src/annotator/sidebar.js
@@ -121,7 +121,7 @@ export default class Sidebar {
     this.toolbar = new ToolbarController(toolbarContainer, {
       createAnnotation: () => guest.createAnnotation(),
       setSidebarOpen: open => (open ? this.open() : this.close()),
-      setHighlightsVisible: show => this.setAllVisibleHighlights(show),
+      setHighlightsVisible: show => this.setHighlightsVisible(show),
     });
 
     if (config.theme === 'clean') {
@@ -130,9 +130,6 @@ export default class Sidebar {
       this.toolbar.useMinimalControls = false;
     }
 
-    this._emitter.subscribe('highlightsVisibleChanged', visible => {
-      this.toolbar.highlightsVisible = visible;
-    });
     this._emitter.subscribe('hasSelectionChanged', hasSelection => {
       this.toolbar.newAnnotationType = hasSelection ? 'annotation' : 'note';
     });
@@ -197,6 +194,12 @@ export default class Sidebar {
         this.iframeContainer.style.display = '';
       }
 
+      // Set initial highlight visibility. We do this only once the sidebar app
+      // is ready because `setHighlightsVisible` needs to reflect this state to
+      // the sidebar app.
+      const showHighlights = config.showHighlights === 'always';
+      this.setHighlightsVisible(showHighlights);
+
       if (
         config.openSidebar ||
         config.annotations ||
@@ -449,7 +452,7 @@ export default class Sidebar {
     this.toolbar.sidebarOpen = true;
 
     if (this.options.showHighlights === 'whenSidebarOpen') {
-      this.guest.setVisibleHighlights(true);
+      this.setHighlightsVisible(true);
     }
 
     this._notifyOfLayoutChange(true);
@@ -464,19 +467,22 @@ export default class Sidebar {
     this.toolbar.sidebarOpen = false;
 
     if (this.options.showHighlights === 'whenSidebarOpen') {
-      this.guest.setVisibleHighlights(false);
+      this.setHighlightsVisible(false);
     }
 
     this._notifyOfLayoutChange(false);
   }
 
   /**
-   * Hide or show highlights associated with annotations in the document.
+   * Set whether highlights are visible in guest frames.
    *
-   * @param {boolean} shouldShowHighlights
+   * @param {boolean} visible
    */
-  setAllVisibleHighlights(shouldShowHighlights) {
-    this._sidebarRPC.call('setVisibleHighlights', shouldShowHighlights);
+  setHighlightsVisible(visible) {
+    this.toolbar.highlightsVisible = visible;
+
+    // Notify sidebar app of change which will in turn reflect state to guest frames.
+    this._sidebarRPC.call('setHighlightsVisible', visible);
   }
 
   /**
diff --git a/src/annotator/test/guest-test.js b/src/annotator/test/guest-test.js
index 25a1345d2ea..e15ff0db7fc 100644
--- a/src/annotator/test/guest-test.js
+++ b/src/annotator/test/guest-test.js
@@ -86,7 +86,6 @@ describe('Guest', () => {
       createChannel: sinon.stub(),
       destroy: sinon.stub(),
       on: sinon.stub(),
-      onConnect: sinon.stub(),
     };
     FakeBridge = sinon.stub().returns(fakeBridge);
 
@@ -406,18 +405,18 @@ describe('Guest', () => {
       });
     });
 
-    describe('on "setVisibleHighlights" event', () => {
+    describe('on "setHighlightsVisible" event', () => {
       it('sets visibility of highlights in document', () => {
         const guest = createGuest();
 
-        emitSidebarEvent('setVisibleHighlights', true);
+        emitSidebarEvent('setHighlightsVisible', true);
         assert.calledWith(
           highlighter.setHighlightsVisible,
           guest.element,
           true
         );
 
-        emitSidebarEvent('setVisibleHighlights', false);
+        emitSidebarEvent('setHighlightsVisible', false);
         assert.calledWith(
           highlighter.setHighlightsVisible,
           guest.element,
@@ -436,7 +435,7 @@ describe('Guest', () => {
     beforeEach(() => {
       fakeSidebarFrame = null;
       guest = createGuest();
-      guest.setVisibleHighlights(true);
+      guest.setHighlightsVisible(true);
       rootElement = guest.element;
 
       // Create a fake highlight as a target for hover and click events.
@@ -519,7 +518,7 @@ describe('Guest', () => {
     });
 
     it('does not focus or select annotations in the sidebar if highlights are hidden', () => {
-      guest.setVisibleHighlights(false);
+      guest.setHighlightsVisible(false);
 
       fakeHighlight.dispatchEvent(new Event('mouseover', { bubbles: true }));
       fakeHighlight.dispatchEvent(new Event('mouseup', { bubbles: true }));
diff --git a/src/annotator/test/sidebar-test.js b/src/annotator/test/sidebar-test.js
index 6bcc1bd1410..c4c9d934d24 100644
--- a/src/annotator/test/sidebar-test.js
+++ b/src/annotator/test/sidebar-test.js
@@ -89,7 +89,7 @@ describe('Sidebar', () => {
         this.contentContainer = sinon.stub().returns(document.body);
         this.createAnnotation = sinon.stub();
         this.fitSideBySide = sinon.stub();
-        this.setVisibleHighlights = sinon.stub();
+        this.setHighlightsVisible = sinon.stub();
       }
     }
     fakeGuest = new FakeGuest();
@@ -244,14 +244,14 @@ describe('Sidebar', () => {
 
     it('shows or hides highlights when toolbar button is clicked', () => {
       const sidebar = createSidebar();
-      sinon.stub(sidebar, 'setAllVisibleHighlights');
+      sinon.stub(sidebar, 'setHighlightsVisible');
 
       FakeToolbarController.args[0][1].setHighlightsVisible(true);
-      assert.calledWith(sidebar.setAllVisibleHighlights, true);
-      sidebar.setAllVisibleHighlights.resetHistory();
+      assert.calledWith(sidebar.setHighlightsVisible, true);
+      sidebar.setHighlightsVisible.resetHistory();
 
       FakeToolbarController.args[0][1].setHighlightsVisible(false);
-      assert.calledWith(sidebar.setAllVisibleHighlights, false);
+      assert.calledWith(sidebar.setHighlightsVisible, false);
     });
 
     it('creates an annotation when toolbar button is clicked', () => {
@@ -562,13 +562,17 @@ describe('Sidebar', () => {
     it('shows highlights if "showHighlights" is set to "whenSidebarOpen"', () => {
       const sidebar = createSidebar({ showHighlights: 'whenSidebarOpen' });
       sidebar.open();
-      assert.calledWith(sidebar.guest.setVisibleHighlights, true);
+      assert.calledWith(fakeBridge.call, 'setHighlightsVisible', true);
     });
 
     it('does not show highlights otherwise', () => {
       const sidebar = createSidebar({ showHighlights: 'never' });
       sidebar.open();
-      assert.notCalled(sidebar.guest.setVisibleHighlights);
+
+      const call = fakeBridge.call
+        .getCalls()
+        .find(args => args[0] === 'setHighlightsVisible' && args[1] === true);
+      assert.isUndefined(call);
     });
 
     it('updates the `sidebarOpen` property of the toolbar', () => {
@@ -585,7 +589,7 @@ describe('Sidebar', () => {
       sidebar.open();
       sidebar.close();
 
-      assert.calledWith(sidebar.guest.setVisibleHighlights, false);
+      assert.calledWith(fakeBridge.call, 'setHighlightsVisible', false);
     });
 
     it('updates the `sidebarOpen` property of the toolbar', () => {
@@ -598,12 +602,25 @@ describe('Sidebar', () => {
     });
   });
 
-  describe('#setAllVisibleHighlights', () =>
+  describe('#setHighlightsVisible', () => {
     it('requests sidebar to set highlight visibility in guest frames', () => {
       const sidebar = createSidebar();
-      sidebar.setAllVisibleHighlights(true);
-      assert.calledWith(fakeBridge.call, 'setVisibleHighlights', true);
-    }));
+      sidebar.setHighlightsVisible(true);
+      assert.calledWith(fakeBridge.call, 'setHighlightsVisible', true);
+
+      sidebar.setHighlightsVisible(false);
+      assert.calledWith(fakeBridge.call, 'setHighlightsVisible', false);
+    });
+
+    it('toggles "Show highlights" control in toolbar', () => {
+      const sidebar = createSidebar();
+      sidebar.setHighlightsVisible(true);
+      assert.isTrue(fakeToolbar.highlightsVisible);
+
+      sidebar.setHighlightsVisible(false);
+      assert.isFalse(fakeToolbar.highlightsVisible);
+    });
+  });
 
   it('hides toolbar controls when using the "clean" theme', () => {
     createSidebar({ theme: 'clean' });
diff --git a/src/sidebar/services/frame-sync.js b/src/sidebar/services/frame-sync.js
index c51c95a22cf..7af5086b2cc 100644
--- a/src/sidebar/services/frame-sync.js
+++ b/src/sidebar/services/frame-sync.js
@@ -78,6 +78,9 @@ export class FrameSyncService {
     // Set of tags of annotations that are currently loaded into the frame
     const inFrame = new Set();
 
+    /** Whether highlights are visible in guest frames. */
+    this._highlightsVisible = false;
+
     /**
      * Watch for changes to the set of annotations displayed in the sidebar and
      * notify connected guests about new/updated/deleted annotations.
@@ -226,6 +229,9 @@ export class FrameSyncService {
      * @param {PortRPC} channel
      */
     const addFrame = channel => {
+      // Synchronize highlight visibility in this guest with the sidebar's controls.
+      channel.call('setHighlightsVisible', this._highlightsVisible);
+
       channel.call('getDocumentInfo', (err, info) => {
         if (err) {
           channel.destroy();
@@ -279,8 +285,9 @@ export class FrameSyncService {
 
     // When user toggles the highlight visibility control in the sidebar container,
     // update the visibility in all the guest frames.
-    this._hostRPC.on('setVisibleHighlights', state => {
-      this._guestRPC.call('setVisibleHighlights', state);
+    this._hostRPC.on('setHighlightsVisible', visible => {
+      this._highlightsVisible = visible;
+      this._guestRPC.call('setHighlightsVisible', visible);
     });
 
     // Create channel for sidebar <-> host communication and send port to host.
diff --git a/src/sidebar/services/test/frame-sync-test.js b/src/sidebar/services/test/frame-sync-test.js
index 428574a18af..26b8c02624d 100644
--- a/src/sidebar/services/test/frame-sync-test.js
+++ b/src/sidebar/services/test/frame-sync-test.js
@@ -426,14 +426,17 @@ describe('FrameSyncService', () => {
 
   context('when a new frame connects', () => {
     let frameInfo;
-    const fakeChannel = {
-      call: function (name, callback) {
-        callback(null, frameInfo);
-      },
-      destroy: sinon.stub(),
-    };
+    let fakeChannel;
 
     beforeEach(() => {
+      fakeChannel = {
+        call: sinon.spy((name, callback) => {
+          if (name === 'getDocumentInfo') {
+            callback(null, frameInfo);
+          }
+        }),
+        destroy: sinon.stub(),
+      };
       frameSync.connect();
     });
 
@@ -450,13 +453,27 @@ describe('FrameSyncService', () => {
     });
 
     it('closes the channel and does not add frame to store if getting document info fails', () => {
-      fakeChannel.call = (name, callback) => callback('Something went wrong');
+      fakeChannel.call = (name, callback) => {
+        if (name === 'getDocumentInfo') {
+          callback('Something went wrong');
+        }
+      };
 
       guestBridge().emit('connect', fakeChannel);
 
       assert.called(fakeChannel.destroy);
       assert.notCalled(fakeStore.connectFrame);
     });
+
+    it("synchronizes highlight visibility in the guest with the sidebar's controls", () => {
+      hostBridge().emit('setHighlightsVisible', true);
+      guestBridge().emit('connect', fakeChannel);
+      assert.calledWith(fakeChannel.call, 'setHighlightsVisible', true);
+
+      hostBridge().emit('setHighlightsVisible', false);
+      guestBridge().emit('connect', fakeChannel);
+      assert.calledWith(fakeChannel.call, 'setHighlightsVisible', false);
+    });
   });
 
   context('when a frame is destroyed', () => {
@@ -528,10 +545,10 @@ describe('FrameSyncService', () => {
       assert.calledWith(hostBridge().call, 'closeSidebar');
     });
 
-    it('calls "setVisibleHighlights"', () => {
-      hostBridge().emit('setVisibleHighlights');
+    it('calls "setHighlightsVisible"', () => {
+      hostBridge().emit('setHighlightsVisible');
 
-      assert.calledWith(guestBridge().call, 'setVisibleHighlights');
+      assert.calledWith(guestBridge().call, 'setHighlightsVisible');
     });
   });
 

From 4a10862f7448c2dd54462d3b1458cd60d66fc838 Mon Sep 17 00:00:00 2001
From: Robert Knight <robertknight@gmail.com>
Date: Thu, 14 Oct 2021 11:39:10 +0100
Subject: [PATCH 3/4] Make showing highlights when creating an annotation work
 for all guests

Change the logic for making highlights visible when creating a new
annotation, so that it works if the annotation was created in a non-host
frame.

This is done by moving the logic from the `beforeAnnotationCreated`
handler in the Sidebar class, which was only called for annotations in
the host frame, to the `beforeCreateAnnotation` handler in
FrameSyncService, which is called for all frames. The sidebar app will
then send a request to show highlights to the host frame, which will
update the sidebar's controls and then relay the request to guest
frames.
---
 src/annotator/sidebar.js                     |  3 +++
 src/annotator/test/sidebar-test.js           |  9 +++++++++
 src/sidebar/services/frame-sync.js           |  5 +++++
 src/sidebar/services/test/frame-sync-test.js | 12 +++++++++++-
 src/types/bridge-events.d.ts                 |  9 +++++++--
 5 files changed, 35 insertions(+), 3 deletions(-)

diff --git a/src/annotator/sidebar.js b/src/annotator/sidebar.js
index e24413ba199..46c53a9f4c4 100644
--- a/src/annotator/sidebar.js
+++ b/src/annotator/sidebar.js
@@ -239,6 +239,9 @@ export default class Sidebar {
     sidebarTrigger(document.body, () => this.open());
     features.init(this._sidebarRPC);
 
+    this._sidebarRPC.on('showHighlights', () =>
+      this.setHighlightsVisible(true)
+    );
     this._sidebarRPC.on('openSidebar', () => this.open());
     this._sidebarRPC.on('closeSidebar', () => this.close());
 
diff --git a/src/annotator/test/sidebar-test.js b/src/annotator/test/sidebar-test.js
index c4c9d934d24..56bfb178c89 100644
--- a/src/annotator/test/sidebar-test.js
+++ b/src/annotator/test/sidebar-test.js
@@ -294,6 +294,15 @@ describe('Sidebar', () => {
       return result;
     };
 
+    describe('on "showHighlights" event', () => {
+      it('makes all highlights visible', () => {
+        createSidebar();
+        assert.isFalse(fakeToolbar.highlightsVisible);
+        emitEvent('showHighlights');
+        assert.isTrue(fakeToolbar.highlightsVisible);
+      });
+    });
+
     describe('on "open" event', () =>
       it('opens the frame', () => {
         const target = sandbox.stub(Sidebar.prototype, 'open');
diff --git a/src/sidebar/services/frame-sync.js b/src/sidebar/services/frame-sync.js
index 7af5086b2cc..0fdf075485b 100644
--- a/src/sidebar/services/frame-sync.js
+++ b/src/sidebar/services/frame-sync.js
@@ -167,6 +167,11 @@ export class FrameSyncService {
           this._hostRPC.call('openSidebar');
         }
 
+        // Ensure that the highlight for the newly-created annotation is visible.
+        // Currently we only support a single, shared visibility state for all highlights
+        // in all frames, so this will make all existing highlights visible too.
+        this._hostRPC.call('showHighlights');
+
         // Create the new annotation in the sidebar.
         annotationsService.create(annot);
       });
diff --git a/src/sidebar/services/test/frame-sync-test.js b/src/sidebar/services/test/frame-sync-test.js
index 26b8c02624d..7832864113a 100644
--- a/src/sidebar/services/test/frame-sync-test.js
+++ b/src/sidebar/services/test/frame-sync-test.js
@@ -310,6 +310,16 @@ describe('FrameSyncService', () => {
   });
 
   context('when a new annotation is created in the frame', () => {
+    it('makes the new highlight visible in the frame', () => {
+      frameSync.connect();
+      fakeStore.isLoggedIn.returns(true);
+      const ann = { target: [] };
+
+      guestBridge().emit('beforeCreateAnnotation', { tag: 't1', msg: ann });
+
+      assert.calledWith(hostBridge().call, 'showHighlights');
+    });
+
     context('when an authenticated user is present', () => {
       it('creates the annotation in the sidebar', () => {
         frameSync.connect();
@@ -344,7 +354,7 @@ describe('FrameSyncService', () => {
 
         guestBridge().emit('beforeCreateAnnotation', { tag: 't1', msg: ann });
 
-        assert.notCalled(hostBridge().call);
+        assert.neverCalledWith(hostBridge().call, 'openSidebar');
       });
     });
 
diff --git a/src/types/bridge-events.d.ts b/src/types/bridge-events.d.ts
index 398bfce45d8..afbd4c5a359 100644
--- a/src/types/bridge-events.d.ts
+++ b/src/types/bridge-events.d.ts
@@ -15,7 +15,7 @@ export type HostToSidebarEvent =
   /**
    * Highlights have been toggled on/off.
    */
-  | 'setVisibleHighlights'
+  | 'setHighlightsVisible'
 
   /**
    * The host informs the sidebar that the sidebar has been opened.
@@ -93,7 +93,7 @@ export type SidebarToGuestEvent =
   /**
    * The sidebar relays to the guest(s) to set the annotation highlights on/off.
    */
-  | 'setVisibleHighlights';
+  | 'setHighlightsVisible';
 
 /**
  * Events that the sidebar sends to the host
@@ -142,6 +142,11 @@ export type SidebarToHostEvent =
    */
   | 'openSidebar'
 
+  /**
+   * The sidebar requests the host to enable the "Show highlights" control.
+   */
+  | 'showHighlights'
+
   /**
    * The sidebar is asking the host to open the partner site profile page.
    * https://h.readthedocs.io/projects/client/en/latest/publishers/config/#cmdoption-arg-onprofilerequest

From 7f2c0b8569c9f6f4e26b68885c402b4f9028fbec Mon Sep 17 00:00:00 2001
From: Robert Knight <robertknight@gmail.com>
Date: Tue, 19 Oct 2021 14:12:33 +0100
Subject: [PATCH 4/4] Simplify a test assertion using `neverCalledWith`

---
 src/annotator/test/sidebar-test.js | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/src/annotator/test/sidebar-test.js b/src/annotator/test/sidebar-test.js
index 56bfb178c89..9412e61ae17 100644
--- a/src/annotator/test/sidebar-test.js
+++ b/src/annotator/test/sidebar-test.js
@@ -577,11 +577,7 @@ describe('Sidebar', () => {
     it('does not show highlights otherwise', () => {
       const sidebar = createSidebar({ showHighlights: 'never' });
       sidebar.open();
-
-      const call = fakeBridge.call
-        .getCalls()
-        .find(args => args[0] === 'setHighlightsVisible' && args[1] === true);
-      assert.isUndefined(call);
+      assert.neverCalledWith(fakeBridge.call, 'setHighlightsVisible');
     });
 
     it('updates the `sidebarOpen` property of the toolbar', () => {