Update review changes (Math.round + leaving width: 0)

GoogleChrome · Mar 22, 2023 · 2e04700 · 2e04700
1 parent 514b647
commit 2e04700
Show file tree

Hide file tree

Showing 2 changed files with 11 additions and 12 deletions.
diff --git a/core/gather/gatherers/full-page-screenshot.js b/core/gather/gatherers/full-page-screenshot.js
@@ -68,15 +68,14 @@ class FullPageScreenshot extends FRGatherer {
     const session = context.driver.defaultSession;
     const metrics = await session.sendCommand('Page.getLayoutMetrics');
 
-    // To obtain a full page screenshot, we resize the emulated viewport to
-    // (1) a height equal to the maximum between visual-viewport height and scaled document height.
-    // (2) a width equal to emulated visual-viewport width (we choose to clip overflow on x-axis).
-    // Finally, we cap the viewport to the maximum size allowance of WebP format.
-    const fullHeight = Math.max(
+    // To obtain a full page screenshot, we resize the emulated viewport height to
+    // the maximum between visual-viewport height and the scaled document height.
+    // Final height is capped to the maximum size allowance of WebP format.
+    // Height needs to be rounded to an integer for Emulation.setDeviceMetricOverrides.
+    const fullHeight = Math.round(Math.max(
       deviceMetrics.height, metrics.cssContentSize.height * metrics.cssVisualViewport.scale
-    );
+    ));
     const height = Math.min(fullHeight, MAX_WEBP_SIZE);
-    const width = Math.min(deviceMetrics.width, MAX_WEBP_SIZE);
 
     // Setup network monitor before we change the viewport.
     const networkMonitor = new NetworkMonitor(context.driver.targetManager);
@@ -93,7 +92,7 @@ class FullPageScreenshot extends FRGatherer {
       mobile: deviceMetrics.mobile,
       deviceScaleFactor: 1,
       height,
-      width,
+      width: 0, // Leave width unchanged
     });
 
     // Now that the viewport is taller, give the page some time to fetch new resources that
@@ -233,7 +232,7 @@ class FullPageScreenshot extends FRGatherer {
             mobile: deviceMetrics.mobile,
             deviceScaleFactor: deviceMetrics.deviceScaleFactor,
             height: deviceMetrics.height,
-            width: deviceMetrics.width,
+            width: 0, // Leave width unchanged
           });
         }
       }

diff --git a/core/test/gather/gatherers/full-page-screenshot-test.js b/core/test/gather/gatherers/full-page-screenshot-test.js
@@ -147,7 +147,7 @@ describe('FullPageScreenshot gatherer', () => {
         mobile: true,
         deviceScaleFactor: 1,
         height: 1500,
-        width: 500,
+        width: 0,
       })
     );
 
@@ -158,7 +158,7 @@ describe('FullPageScreenshot gatherer', () => {
         mobile: true,
         deviceScaleFactor: 2,
         height: 500,
-        width: 500,
+        width: 0,
       })
     );
   });
@@ -186,7 +186,7 @@ describe('FullPageScreenshot gatherer', () => {
       {
         mobile: true,
         deviceScaleFactor: 1,
-        width: 412, // Horizontal overflow will be clipped to screen size.
+        width: 0,
         height: 16383,
       }
     );