Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix the onboarding flow for the Chrome MV3 build #2509

Merged
merged 1 commit into from
Apr 17, 2024

Conversation

kzar
Copy link
Collaborator

@kzar kzar commented Apr 16, 2024

The onboarding code has some logic to display a welcome banner the
first time the user performs a DuckDuckGo search after installing the
extension. Until now, that code did not work for Chrome MV3 builds,
since it attempted to inject a script into the "main world" by
appending a <script> element - that does not work with MV3 due to the
stricter Content-Security-Policy used. Let's use the
scripting.executeScript API instead to inject the required code into
the "main world".

Note: This code is somewhat convoluted and could likely be simplified
in the future. But for now, with the upcoming MV3 migration
deadline, let's make the minimum changes required to get this
working!

Reviewer: @sballesteros, @sammacbeth

Steps to test this PR:

  1. Install the extension
  2. Ensure DDG is the default browser if you're running Linux
  3. Perform a search from the URL bar
  4. Ensure the purple onboarding banner is displayed and a CSP warning isn't shown in the console
  5. Perform another search from the URL bar and ensure the onboarding banner is not displayed this time

Note: Test both with MV2 and MV3 builds of the extension!

@kzar kzar force-pushed the mv3-fix-onboarding branch 4 times, most recently from 7ae6698 to 775ae6f Compare April 16, 2024 13:14
@kzar kzar marked this pull request as ready for review April 16, 2024 13:19
@kzar kzar marked this pull request as draft April 16, 2024 13:29
The onboarding code has some logic to display a welcome banner the
first time the user performs a DuckDuckGo search after installing the
extension. Until now, that code did not work for Chrome MV3 builds,
since it attempted to inject a script into the "main world" by
appending a <script> element - that does not work with MV3 due to the
stricter Content-Security-Policy used. Let's use the
scripting.executeScript API instead to inject the required code into
the "main world".

Note: This code is somewhat convoluted and could likely be simplified
      in the future. But for now, with the upcoming MV3 migration
      deadline, let's make the minimum changes required to get this
      working!
@kzar kzar marked this pull request as ready for review April 16, 2024 15:37
Copy link
Collaborator

@sammacbeth sammacbeth left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This worked for me in testing.

I think it would be helpful to file a follow-up to tidying up this code (post MV3 migration):

  • Make it a standalone 'component' (to move listeners of our events.js), and keep code in one place.
  • Use build flags to only include the component on chrome builds (i.e. BUILD_TARGET !== 'firefox').

@kzar kzar merged commit 2e4441d into duckduckgo:main Apr 17, 2024
23 of 24 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants