Skip to content

Commit

Permalink
Ensure onFirstSearchPostExtensionInstallCalls is called correctly (#2513
Browse files Browse the repository at this point in the history
)

When fixing the onboarding flow for the Chrome MV3 build[1], I made a
mistake. For MV3 builds, the page's onFirstSearchPostExtensionInstall
function was being called like:

    onFirstSearchPostExtensionInstall({
        isAddressBarQuery: false,
        showCounterMessaging: true,
        showWelcomeBanner: true
    }, {
        hadFocusOnStart: true
    })

Instead of like:

    onFirstSearchPostExtensionInstall({
        hadFocusOnStart: true,
        isAddressBarQuery: false,
        showCounterMessaging: true,
        showWelcomeBanner: true
    })

(I had missed a subtle Object.assign call.)

Let's fix that here, and also add test coverage for that call to catch
similar mistakes in the future.

1 - 2e4441d
  • Loading branch information
kzar authored Apr 24, 2024
1 parent bfbc6bb commit a8144d6
Show file tree
Hide file tree
Showing 2 changed files with 42 additions and 4 deletions.
36 changes: 36 additions & 0 deletions integration-test/onboarding.spec.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,22 @@
import { test, expect } from './helpers/playwrightHarness'
import backgroundWait from './helpers/backgroundWait'

function stubOnFirstSearchPostExtensionInstallOnInit (page) {
return page.addInitScript(() => {
window.afterInitCall = new Promise(resolve => {
Object.defineProperty(
window,
'onFirstSearchPostExtensionInstall',
{
value (...args) {
resolve(JSON.stringify(args))
}
}
)
})
})
}

test.describe('onboarding', () => {
test('should manage the onboarding state and inject a script that calls window.onFirstSearchPostExtensionInstall on the first search post extension', async ({ manifestVersion, context, backgroundPage, page }) => {
await backgroundWait.forExtensionLoaded(context)
Expand All @@ -15,6 +31,7 @@ test.describe('onboarding', () => {
expect(params.showWelcomeBanner).toBe(true)
expect(params.showCounterMessaging).toBe(true)

await stubOnFirstSearchPostExtensionInstallOnInit(page)
await page.bringToFront()
await page.goto('https://duckduckgo.com/?q=hello')

Expand All @@ -26,6 +43,15 @@ test.describe('onboarding', () => {
expect(hasScriptHandle).toBeTruthy()
}

expect(
JSON.parse(await page.evaluate(() => window.afterInitCall))
).toEqual([{
hadFocusOnStart: true,
isAddressBarQuery: false,
showCounterMessaging: true,
showWelcomeBanner: true
}])

const nextParams = await backgroundPage.evaluate(() => {
return {
showWelcomeBanner: globalThis.dbg.settings.getSetting('showWelcomeBanner'),
Expand Down Expand Up @@ -64,6 +90,7 @@ test.describe('onboarding', () => {
test('should allow the site to reschedule the counter messaging (Chrome only)', async ({ context, backgroundPage, page }) => {
await backgroundWait.forExtensionLoaded(context)

await stubOnFirstSearchPostExtensionInstallOnInit(page)
await page.goto('https://duckduckgo.com/?q=hello')

await page.evaluate(() => {
Expand All @@ -75,5 +102,14 @@ test.describe('onboarding', () => {
return globalThis.dbg.settings.getSetting('rescheduleCounterMessagingOnStart')
})
expect(rescheduleCounterMessagingOnStart).toBe(true)

expect(
JSON.parse(await page.evaluate(() => window.afterInitCall))
).toEqual([{
hadFocusOnStart: true,
isAddressBarQuery: false,
showCounterMessaging: true,
showWelcomeBanner: true
}])
})
})
10 changes: 6 additions & 4 deletions shared/js/background/onboarding.js
Original file line number Diff line number Diff line change
Expand Up @@ -121,10 +121,12 @@ function onDocumentEndMainWorld ({

if (window.onFirstSearchPostExtensionInstall) {
const { documentStartData } = e.data
window.onFirstSearchPostExtensionInstall(
{ isAddressBarQuery, showWelcomeBanner, showCounterMessaging },
documentStartData
)
window.onFirstSearchPostExtensionInstall({
isAddressBarQuery,
showWelcomeBanner,
showCounterMessaging,
...documentStartData
})
}
}
})
Expand Down

0 comments on commit a8144d6

Please sign in to comment.