Skip to content

Commit 589ff45

Browse files
committed
fix: revert viewport rounding changes and fix scrollElementIntoView
- Revert injectWebviewOverlay.ts to use Math.round() for viewport data collection, fixing ~0.2% fullpage stitching mismatches on Android - Revert getMobileViewPortPosition to main's calculation logic, removing Math.min clamping that caused viewport dimension inconsistencies - Fix scrollElementIntoView to account for current scroll position when computing target scroll offset (BCR.top is viewport-relative, not document-absolute), preventing blank iOS element screenshots - Update unit tests and regenerate affected Android fullpage baselines Made-with: Cursor
1 parent 8d24804 commit 589ff45

12 files changed

Lines changed: 39 additions & 54 deletions

packages/image-comparison-core/src/clientSideScripts/injectWebviewOverlay.test.ts

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,7 @@ describe('injectWebviewOverlay', () => {
6767
})
6868
})
6969

70-
it('should preserve float precision with non-integer DPR (Android)', () => {
70+
it('should round values to integers with non-integer DPR (Android)', () => {
7171
Object.defineProperty(window, 'devicePixelRatio', {
7272
value: 2.625,
7373
configurable: true,
@@ -94,10 +94,10 @@ describe('injectWebviewOverlay', () => {
9494
const parsedData = JSON.parse(overlay.dataset.icsWebviewData!)
9595

9696
expect(parsedData).toEqual({
97-
x: 206 * 2.625,
98-
y: 181 * 2.625,
99-
width: 412 * 2.625,
100-
height: 363 * 2.625,
97+
x: Math.round(206 * 2.625),
98+
y: Math.round(181 * 2.625),
99+
width: Math.round(412 * 2.625),
100+
height: Math.round(363 * 2.625),
101101
})
102102
})
103103

packages/image-comparison-core/src/clientSideScripts/injectWebviewOverlay.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -21,10 +21,10 @@ export function injectWebviewOverlay(isAndroid: boolean): void {
2121
overlay.onclick = (event) => {
2222
const { clientX: x, clientY: y } = event
2323
const data = {
24-
x: x * dpr,
25-
y: y * dpr,
26-
width: window.innerWidth * dpr,
27-
height: document.documentElement.clientHeight * dpr,
24+
x: Math.round(x * dpr),
25+
y: Math.round(y * dpr),
26+
width: Math.round(window.innerWidth * dpr),
27+
height: Math.round(document.documentElement.clientHeight * dpr),
2828
}
2929

3030
overlay.dataset.icsWebviewData = JSON.stringify(data)

packages/image-comparison-core/src/clientSideScripts/scrollElementIntoView.test.ts

Lines changed: 2 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,7 @@ describe('scrollElementIntoView', () => {
6969
const result = scrollElementIntoView(mockElement, addressBarShadowPadding)
7070

7171
expect(result).toBe(50)
72-
// Document y = currentScroll(50) + BCR.top(100) - padding(10) = 140
72+
// currentPosition (50) + BCR.top (100) - padding (10) = 140
7373
expect(mockHtmlNode.scrollTop).toBe(140)
7474
})
7575

@@ -81,21 +81,10 @@ describe('scrollElementIntoView', () => {
8181
const result = scrollElementIntoView(mockElement, addressBarShadowPadding)
8282

8383
expect(result).toBe(50)
84-
// Document y = currentScroll(50) + BCR.top(100) - padding(10) = 140
84+
// currentPosition (50) + BCR.top (100) - padding (10) = 140
8585
expect(mockBodyNode.scrollTop).toBe(140)
8686
})
8787

88-
it('should not re-scroll when element is already at the viewport top', () => {
89-
mockHtmlNode.scrollTop = 600
90-
;(mockElement.getBoundingClientRect as ReturnType<typeof vi.fn>).mockReturnValue({ top: 0 })
91-
const addressBarShadowPadding = 0
92-
const result = scrollElementIntoView(mockElement, addressBarShadowPadding)
93-
94-
expect(result).toBe(600)
95-
// Document y = currentScroll(600) + BCR.top(0) - padding(0) = 600 (stays put)
96-
expect(mockHtmlNode.scrollTop).toBe(600)
97-
})
98-
9988
it('should not scroll when neither html nor body is scrollable', () => {
10089
Object.defineProperty(mockHtmlNode, 'scrollHeight', { value: 500 })
10190
Object.defineProperty(mockBodyNode, 'scrollHeight', { value: 500 })

packages/image-comparison-core/src/helpers/utils.test.ts

Lines changed: 14 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -691,7 +691,7 @@ describe('utils', () => {
691691
expect(result).toMatchSnapshot()
692692
})
693693

694-
it('should handle float overlay values from non-integer DPR and ensure consistent dimensions', async () => {
694+
it('should handle rounded overlay values from non-integer DPR', async () => {
695695
const dpr = 2.625
696696
const screenW = 1080
697697
const screenH = 2424
@@ -701,17 +701,20 @@ describe('utils', () => {
701701
const cssWidth = 412
702702
const cssHeight = 363
703703

704+
const overlayWidth = Math.round(cssWidth * dpr)
705+
const overlayHeight = Math.round(cssHeight * dpr)
706+
704707
vi.mocked(mockBrowserInstance.execute)
705708
.mockResolvedValueOnce(undefined) // loadBase64Html
706709
.mockResolvedValueOnce(undefined) // checkMetaTag (iOS)
707710
.mockResolvedValueOnce(undefined) // injectWebviewOverlay
708711
.mockResolvedValueOnce(undefined) // executeNativeClick
709712
.mockResolvedValueOnce({
710-
x: cssClickX * dpr,
711-
y: cssClickY * dpr,
712-
width: cssWidth * dpr,
713-
height: cssHeight * dpr,
714-
}) // getMobileWebviewClickAndDimensions (floats, not rounded)
713+
x: Math.round(cssClickX * dpr),
714+
y: Math.round(cssClickY * dpr),
715+
width: overlayWidth,
716+
height: overlayHeight,
717+
}) // getMobileWebviewClickAndDimensions (rounded integers from overlay)
715718

716719
const result = await getMobileViewPortPosition({
717720
browserInstance: mockBrowserInstance,
@@ -720,18 +723,14 @@ describe('utils', () => {
720723
screenWidth: screenW,
721724
})
722725

723-
const viewportTop = Math.max(0, Math.round(screenH / 2 - cssClickY * dpr))
724-
const viewportLeft = Math.max(0, Math.round(screenW / 2 - cssClickX * dpr))
725-
const viewportWidth = Math.min(Math.round(cssWidth * dpr), screenW - viewportLeft)
726-
const viewportHeight = Math.min(Math.round(cssHeight * dpr), screenH - viewportTop)
726+
const viewportTop = Math.max(0, Math.round(screenH / 2 - Math.round(cssClickY * dpr)))
727+
const viewportLeft = Math.max(0, Math.round(screenW / 2 - Math.round(cssClickX * dpr)))
727728

728729
expect(result.viewport.y).toBe(viewportTop)
729730
expect(result.viewport.x).toBe(viewportLeft)
730-
expect(result.viewport.width).toBe(viewportWidth)
731-
expect(result.viewport.height).toBe(viewportHeight)
732-
expect(result.statusBarAndAddressBar.height).toBe(viewportTop)
733-
expect(result.viewport.y + result.viewport.height + result.bottomBar.height).toBe(screenH)
734-
expect(result.viewport.x + result.viewport.width + result.rightSidePadding.width).toBe(screenW)
731+
expect(result.viewport.width).toBe(overlayWidth)
732+
expect(result.viewport.height).toBe(overlayHeight)
733+
expect(result.statusBarAndAddressBar.height).toBe(Math.max(0, Math.round(viewportTop)))
735734
})
736735

737736
it('should return initialDeviceRectangles if not WebView (native context)', async () => {

packages/image-comparison-core/src/helpers/utils.ts

Lines changed: 10 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -526,24 +526,21 @@ export async function getMobileViewPortPosition({
526526
const { y, x, width, height } = await browserInstance.execute(getMobileWebviewClickAndDimensions, '[data-test="ics-overlay"]')
527527
// 4.b reset the url
528528
await browserInstance.url(currentUrl)
529-
// 5. Calculate the position of the viewport based on the click position of the native click vs the overlay.
530-
// The overlay values may be floats (CSS pixels * DPR for Android, CSS pixels for iOS).
531-
// We round edges once and derive dimensions from edges to guarantee
532-
// viewportTop + viewportHeight + bottomBarHeight === screenHeight (no gaps).
529+
// 5. Calculate the position of the viewport based on the click position of the native click vs the overlay
533530
const viewportTop = Math.max(0, Math.round(nativeClickY - y))
534531
const viewportLeft = Math.max(0, Math.round(nativeClickX - x))
535-
const viewportWidth = Math.min(Math.round(width), screenWidth - viewportLeft)
536-
const viewportHeight = Math.min(Math.round(height), screenHeight - viewportTop)
537-
const bottomBarHeight = Math.max(0, screenHeight - viewportTop - viewportHeight)
538-
const rightSidePaddingWidth = Math.max(0, screenWidth - viewportLeft - viewportWidth)
532+
const statusBarAndAddressBarHeight = Math.max(0, Math.round(viewportTop))
533+
const bottomBarHeight = Math.max(0, Math.round(screenHeight - (viewportTop + height)))
534+
const leftSidePaddingWidth = Math.max(0, Math.round(viewportLeft))
535+
const rightSidePaddingWidth = Math.max(0, Math.round(screenWidth - (viewportLeft + width)))
539536
const deviceRectangles = {
540537
...initialDeviceRectangles,
541-
bottomBar: { y: viewportTop + viewportHeight, x: 0, width: screenWidth, height: bottomBarHeight },
542-
leftSidePadding: { y: viewportTop, x: 0, width: viewportLeft, height: viewportHeight },
543-
rightSidePadding: { y: viewportTop, x: viewportLeft + viewportWidth, width: rightSidePaddingWidth, height: viewportHeight },
538+
bottomBar: { y: viewportTop + height, x: 0, width: screenWidth, height: bottomBarHeight },
539+
leftSidePadding: { y: viewportTop, x: 0, width: leftSidePaddingWidth, height: height },
540+
rightSidePadding: { y: viewportTop, x: viewportLeft + width, width: rightSidePaddingWidth, height: height },
544541
screenSize: { height: screenHeight, width: screenWidth },
545-
statusBarAndAddressBar: { y: 0, x: 0, width: screenWidth, height: viewportTop },
546-
viewport: { y: viewportTop, x: viewportLeft, width: viewportWidth, height: viewportHeight },
542+
statusBarAndAddressBar: { y: 0, x: 0, width: screenWidth, height: statusBarAndAddressBarHeight },
543+
viewport: { y: viewportTop, x: viewportLeft, width: width, height: height },
547544
}
548545

549546
return deviceRectangles
Loading
Loading
-1.27 KB
Loading
157 Bytes
Loading
-45 Bytes
Loading

0 commit comments

Comments
 (0)