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

Prevent in-context signup showing on local or invalid domain pages #263

Conversation

alistairjcbrown
Copy link
Member

Reviewer: @shakyShane @GioSensation
Asana: https://app.asana.com/0/0/1204038361216231/f

Description

Restrict in-context signup from running on local pages with valid TLD. This branch also updates the in-context signup tests to run on a real domain, mocked in puppeteer.

Steps to test

  1. Build update into extension
  2. Run local server and go to email-autofill.html integration test file
  3. Confirm that in-context signup does not display
  4. Go to https://duckduckgo.com/newsletter
  5. Confirm that in-context signup does display

@alistairjcbrown alistairjcbrown force-pushed the abrown/prevent-incontext-signup-on-local-pages branch from 375e5a6 to a5b808e Compare February 25, 2023 00:25
Copy link
Member

@GioSensation GioSensation left a comment

Choose a reason for hiding this comment

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

Main feedback is to consider using isTestMode() to avoid all the test harness. Let me know what you think.

src/url-utils.js Outdated Show resolved Hide resolved
src/url-utils.js Outdated Show resolved Hide resolved
src/url-utils.js Outdated Show resolved Hide resolved
src/InContextSignup.js Outdated Show resolved Hide resolved
@alistairjcbrown alistairjcbrown force-pushed the abrown/prevent-incontext-signup-on-local-pages branch 2 times, most recently from d6618af to 361d923 Compare February 28, 2023 12:31
@alistairjcbrown alistairjcbrown force-pushed the abrown/prevent-incontext-signup-on-local-pages branch from 361d923 to 813653f Compare February 28, 2023 12:37
@alistairjcbrown
Copy link
Member Author

@GioSensation @shakyShane Integration test setup updated. Any other comments?

@alistairjcbrown alistairjcbrown changed the base branch from main to abrown/changes-to-in-context-signup-treatment February 28, 2023 20:03
@alistairjcbrown alistairjcbrown merged commit 741ed92 into abrown/changes-to-in-context-signup-treatment Feb 28, 2023
@alistairjcbrown alistairjcbrown deleted the abrown/prevent-incontext-signup-on-local-pages branch February 28, 2023 20:04
alistairjcbrown added a commit that referenced this pull request Mar 23, 2023
* Prevent in-context signup showing on local or invalid domain pages (#263)

* Don't open in-context signup on input click (#264)

* Show grayscale Dax icon for in-context signup (#265)

* Remove initial dismissal in favor of single permanent dismissal (#266)

* Only show in-context signup if recently installed (#267)

* Adds a close X button to the top right of the tooltip (#268)

* Fix grayscale dax logo (#271)

* Ema/prevent focus extensions (#270)

* Update icon asset to fix grayscale version (#276)

* Improve narrow width iframes (#273)

* Add Email Protection tooltip pixel (#277)

* Fade Dax icon in on hover for in-context signup (#278)

* Fade Dax icon in on hover for in-context signup

* Keep Dax icon with active styles when tooltip open

* Prevent icon flicker when clicking Dax icon for open tooltip

* Move style management into form

* Position tooltip above input when no space below (#282)

* Position tooltip above input when no space below

* Preload webfonts before showing tooltip

This prevents the fonts loading in once the tooltip is shown and causing
a layout shift. This is more important when the tooltip is showing above
the input, as it can cause the tooltip to jump up.

* Check tooltip position after revealed and bail if called before

When running on slower devices, the tooltip position may be checked
before the tooltip is revealed. When this happens, we now bail early
without updating the position. Check position is now called after styles
have loaded, when we know the tooltip is visible.

* Hide caret initially and reduce position calculation calls

* Read hidden attribute from DOM

Signed-off-by: Emanuele Feliziani <feliziani.emanuele@gmail.com>

* Fix linting issue with playwright report

Signed-off-by: Emanuele Feliziani <feliziani.emanuele@gmail.com>

* Rename typoed file

Signed-off-by: Emanuele Feliziani <feliziani.emanuele@gmail.com>

---------

Signed-off-by: Emanuele Feliziani <feliziani.emanuele@gmail.com>
Co-authored-by: Emanuele Feliziani <feliziani.emanuele@gmail.com>

---------

Signed-off-by: Emanuele Feliziani <feliziani.emanuele@gmail.com>
Co-authored-by: Emanuele Feliziani <feliziani.emanuele@gmail.com>
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.

3 participants