Prevent body left margin from increasing when enabling side-by-side mode

Prevent body from jumping right in cases where `margin: auto` is set on
the body. Body may move left to make room for sidebar, but should never
move to the right.

Fixes #4280

src/annotator/integrations/html.js

@@ -117,9 +117,21 @@ export class HTMLIntegration {
     const padding = 12;
     const rightMargin = sidebarWidth + padding;
 
+    /** @param {HTMLElement} element */
+    const computeLeftMargin = element =>
+      parseInt(window.getComputedStyle(element).marginLeft, 10);
+
     preserveScrollPosition(() => {
       // nb. Adjusting the body size this way relies on the page not setting a
       // width on the body. For sites that do this won't work.
+
+      // Remove any margins we've previously set
+      document.body.style.marginLeft = '';
+      document.body.style.marginRight = '';
+
+      // Keep track of what left margin would be naturally without right margin set
+      const beforeBodyLeft = computeLeftMargin(document.body);
+
       document.body.style.marginRight = `${rightMargin}px`;
 
       const contentArea = guessMainContentArea(document.body);
@@ -135,6 +147,15 @@ export class HTMLIntegration {
           document.body.style.marginRight = `${adjustedMargin}px`;
         }
 
+        // Changes to right margin can affect left margin in cases where body
+        // has `margin:auto`. It's OK to move the body to the left to make
+        // space, but avoid moving it to the right.
+        // See https://github.com/hypothesis/client/issues/4280
+        const afterBodyLeft = computeLeftMargin(document.body);
+        if (afterBodyLeft > beforeBodyLeft) {
+          document.body.style.marginLeft = `${beforeBodyLeft}px`;
+        }
+
         // If the main content appears to be right up against the edge of the
         // window, add padding for readability.
         if (contentArea.left < padding) {

src/annotator/integrations/test/html-test.js

@@ -178,6 +178,96 @@ describe('HTMLIntegration', () => {
         integration.fitSideBySide({ expanded: false });
         assert.deepEqual(getMargins(), [null, null]);
       });
+
+      context('main content area has margin:auto', () => {
+        const bodyWidth = 400;
+        const autoMargin = Math.floor((window.innerWidth - bodyWidth) / 2);
+
+        function getComputedMargins(element) {
+          const leftMargin = Math.floor(
+            parseInt(window.getComputedStyle(element).marginLeft, 10)
+          );
+          const rightMargin = Math.floor(
+            parseInt(window.getComputedStyle(element).marginRight, 10)
+          );
+          return [leftMargin, rightMargin];
+        }
+
+        // Add a style node to set a max-width and auto margins on the body
+        function appendBodyStyles(document_) {
+          const el = document_.createElement('style');
+          el.type = 'text/css';
+          el.textContent = `body { margin: 0 auto; max-width: ${bodyWidth}px }`;
+          el.classList.add('js-style-test');
+          document_.body.appendChild(el);
+        }
+
+        before(() => {
+          appendBodyStyles(document);
+        });
+
+        after(() => {
+          // Remove test styles
+          const elements = document.querySelectorAll('.js-style-test');
+          for (let i = 0; i < elements.length; i++) {
+            elements[i].remove();
+          }
+        });
+
+        beforeEach(() => {
+          // In these tests, we're treating the body element as the
+          // main content area.
+          //
+          // `guessMainContent` area is called _after_ a right margin is set
+          // on the body, so we'll return here the updated computed left and
+          // right position of the body to emulate a real-life result
+          fakeGuessMainContentArea.callsFake(bodyEl => {
+            const margins = getComputedMargins(bodyEl);
+            return { left: margins[0], right: window.innerWidth - margins[1] };
+          });
+        });
+
+        it('should not move the main content to the right', () => {
+          const integration = createIntegration();
+          // Before enabling side-by-side, the horizontal margins on the body
+          // are derived based on `margin: auto` in the stylesheet
+          assert.deepEqual(getComputedMargins(document.body), [
+            autoMargin,
+            autoMargin,
+          ]);
+
+          // Will result in a right margin of 112px (100 + 12 padding)
+          integration.fitSideBySide({ expanded: true, width: 100 });
+
+          // Without intervention, the left margin would have _increased_ to
+          // balance out the remaining space, that is:
+          // innerWidth - bodyWidth - 112 > 200
+          //
+          // To prevent content jumping to the right, implementation sets left
+          // margin to original auto margin
+          assert.deepEqual(getComputedMargins(document.body), [
+            autoMargin,
+            112,
+          ]);
+        });
+
+        it('may move the main content to the left to make room for sidebar', () => {
+          const integration = createIntegration();
+
+          // Will result in right margin of 262 (250 + 12 padding)
+          integration.fitSideBySide({ expanded: true, width: 250 });
+
+          // The amount of space available to the left of the body is now _less_
+          // than the original auto-left-margin. This is fine: let the auto
+          // margin re-adjust to the available amount of space (move to the left):
+          const updatedMargins = getComputedMargins(document.body);
+          const expectedLeftMargin = Math.floor(
+            window.innerWidth - bodyWidth - 262
+          );
+          assert.equal(updatedMargins[0], expectedLeftMargin);
+          assert.isBelow(updatedMargins[0], autoMargin);
+        });
+      });
     });
   });