Skip to content

Commit b934de8

Browse files
committed
fix: enhance fix for stale elements
1 parent 2745baa commit b934de8

3 files changed

Lines changed: 39 additions & 29 deletions

File tree

packages/image-comparison-core/src/methods/rectangles.test.ts

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -774,14 +774,16 @@ describe('rectangles', () => {
774774
desktopOptions.browserInstance = mockBrowserInstance
775775
})
776776

777-
it('should resolve elements via raw BCR on desktop and apply DPR without re-querying', async () => {
777+
it('should resolve elements via raw BCR on desktop and apply DPR', async () => {
778778
const mockElement = { elementId: 'el1', selector: '.nav' } as WebdriverIO.Element
779+
const freshElement = { elementId: 'el1-fresh', selector: '.nav' } as unknown as WebdriverIO.Element
780+
vi.mocked(mockBrowserInstance.$$).mockResolvedValueOnce([freshElement] as any)
779781
mockExecute.mockResolvedValueOnce({ x: 10, y: 20, width: 200, height: 50 })
780782

781783
const result = await determineWebScreenIgnoreRegions(desktopOptions, [mockElement])
782784

785+
expect(mockBrowserInstance.$$).toHaveBeenCalledWith('.nav')
783786
expect(mockExecute).toHaveBeenCalledOnce()
784-
expect(mockBrowserInstance.$).not.toHaveBeenCalled()
785787
expect(result).toEqual([
786788
{ x: 20, y: 40, width: 400, height: 100 },
787789
])
@@ -867,6 +869,7 @@ describe('rectangles', () => {
867869
isAndroidNativeWebScreenshot: true,
868870
}
869871
const mockElement = { elementId: 'el1', selector: '#header' } as WebdriverIO.Element
872+
vi.mocked(mockBrowserInstance.$$).mockResolvedValueOnce([mockElement] as any)
870873
mockExecute.mockResolvedValueOnce({ x: 0, y: 0, width: 412, height: 64 })
871874

872875
const result = await determineWebScreenIgnoreRegions(androidOptions, [mockElement])
@@ -886,6 +889,7 @@ describe('rectangles', () => {
886889
isAndroidNativeWebScreenshot: false,
887890
}
888891
const mockElement = { elementId: 'el1', selector: '#header' } as WebdriverIO.Element
892+
vi.mocked(mockBrowserInstance.$$).mockResolvedValueOnce([mockElement] as any)
889893
mockExecute.mockResolvedValueOnce({ x: 0, y: 0, width: 412, height: 64 })
890894

891895
const result = await determineWebScreenIgnoreRegions(androidChromeOptions, [mockElement])
@@ -909,6 +913,7 @@ describe('rectangles', () => {
909913

910914
it('should handle mixed elements and regions with DPR applied to both', async () => {
911915
const mockElement = { elementId: 'el1', selector: '.ad' } as WebdriverIO.Element
916+
vi.mocked(mockBrowserInstance.$$).mockResolvedValueOnce([mockElement] as any)
912917
const region = { x: 500, y: 0, width: 200, height: 90 }
913918
mockExecute.mockResolvedValueOnce({ x: 10, y: 20, width: 300, height: 80 })
914919

@@ -929,6 +934,8 @@ describe('rectangles', () => {
929934

930935
it('should handle chainable promise elements', async () => {
931936
const chainableElement = Promise.resolve({ elementId: 'el1', selector: '.footer' } as WebdriverIO.Element)
937+
const freshElement = { elementId: 'el1-fresh', selector: '.footer' } as unknown as WebdriverIO.Element
938+
vi.mocked(mockBrowserInstance.$$).mockResolvedValueOnce([freshElement] as any)
932939
mockExecute.mockResolvedValueOnce({ x: 0, y: 900, width: 1200, height: 100 })
933940

934941
const result = await determineWebScreenIgnoreRegions(desktopOptions, [chainableElement as any])
@@ -940,6 +947,7 @@ describe('rectangles', () => {
940947

941948
it('should use floor/ceil rounding on sub-pixel BCR values to fully cover elements', async () => {
942949
const mockElement = { elementId: 'el1', selector: '.banner' } as WebdriverIO.Element
950+
vi.mocked(mockBrowserInstance.$$).mockResolvedValueOnce([mockElement] as any)
943951
// Sub-pixel BCR values that would lose precision if rounded independently
944952
mockExecute.mockResolvedValueOnce({ x: 0.33, y: 50.67, width: 412.5, height: 64.33 })
945953

packages/image-comparison-core/src/methods/rectangles.ts

Lines changed: 21 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -296,37 +296,31 @@ export async function determineWebScreenIgnoreRegions(
296296
const rect = el.getBoundingClientRect()
297297
return { x: rect.x, y: rect.y, width: rect.width, height: rect.height }
298298
}
299+
// Browsers can invalidate element references when the DOM is mutated
300+
// (e.g. by beforeScreenshot CSS/style injection). Re-query via $$ to
301+
// get fresh refs. Use $$ per unique selector so multiple elements
302+
// sharing the same selector (e.g. from a $$ call) each resolve to
303+
// the correct match by index.
299304
const regionsFromElements: RectanglesOutput[] = []
300-
if (isIOS && elements.length > 0) {
301-
// iOS Safari invalidates element references when the DOM is mutated
302-
// (e.g. by beforeScreenshot CSS injection). Re-query to get fresh refs.
303-
// Use $$ per unique selector so multiple elements sharing the same
304-
// selector (e.g. from a $$ call) each resolve to the correct match.
305-
const selectorCache = new Map<string, WebdriverIO.Element[]>()
306-
const selectorIndex = new Map<string, number>()
307-
308-
for (const element of elements) {
309-
const selector = element.selector as string
310-
311-
if (!selectorCache.has(selector)) {
312-
const fresh = await browserInstance.$$(selector)
313-
selectorCache.set(selector, fresh as unknown as WebdriverIO.Element[])
314-
selectorIndex.set(selector, 0)
315-
}
305+
const selectorCache = new Map<string, WebdriverIO.Element[]>()
306+
const selectorIndex = new Map<string, number>()
316307

317-
const idx = selectorIndex.get(selector)!
318-
const cached = selectorCache.get(selector)!
319-
const el = idx < cached.length ? cached[idx] : element
320-
selectorIndex.set(selector, idx + 1)
308+
for (const element of elements) {
309+
const selector = element.selector as string
321310

322-
const bcr = await browserInstance.execute(rawBcr, el as any) as RectanglesOutput
323-
regionsFromElements.push(bcr)
324-
}
325-
} else {
326-
for (const element of elements) {
327-
const bcr = await browserInstance.execute(rawBcr, element as any) as RectanglesOutput
328-
regionsFromElements.push(bcr)
311+
if (!selectorCache.has(selector)) {
312+
const fresh = await browserInstance.$$(selector)
313+
selectorCache.set(selector, fresh as unknown as WebdriverIO.Element[])
314+
selectorIndex.set(selector, 0)
329315
}
316+
317+
const idx = selectorIndex.get(selector)!
318+
const cached = selectorCache.get(selector)!
319+
const el = idx < cached.length ? cached[idx] : element
320+
selectorIndex.set(selector, idx + 1)
321+
322+
const bcr = await browserInstance.execute(rawBcr, el as any) as RectanglesOutput
323+
regionsFromElements.push(bcr)
330324
}
331325

332326
return [...regions, ...regionsFromElements]

tests/specs/mobile.web.spec.ts

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -200,6 +200,14 @@ const skipRules: SkipRule[] = [
200200
orientations: ['landscape', 'portrait'],
201201
reason: 'Fully ignored in the screenshot so it will never find a difference',
202202
},
203+
{
204+
titleIncludes: 'screen successful',
205+
deviceName: 'Pixel 4',
206+
platformName: 'Android',
207+
platformVersions: ['13'],
208+
orientations: ['portrait'],
209+
reason: 'Elements not visible in the screenshot, no value in testing',
210+
},
203211
{
204212
titleIncludes: 'ignore elements',
205213
deviceName: 'Pixel 9 Pro',

0 commit comments

Comments
 (0)