-
Couldn't load subscription status.
- Fork 3.4k
fix: Add new flags and preferences for Chrome #32811
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
Conversation
cypress
|
||||||||||||||||||||||||||||||||||||||||
| Project |
cypress
|
| Branch Review |
new-chrome-flags-prefs
|
| Run status |
|
| Run duration | 20m 13s |
| Commit |
|
| Committer | Jennifer Shehane |
| View all properties for this run ↗︎ | |
| Test results | |
|---|---|
|
|
0
|
|
|
12
|
|
|
1102
|
|
|
4
|
|
|
26755
|
| View all changes introduced in this branch ↗︎ | |
Warning
Partial Report: The results for the Application Quality reports may be incomplete.
UI Coverage
45.48%
|
|
|---|---|
|
|
188
|
|
|
161
|
Accessibility
97.98%
|
|
|---|---|
|
|
4 critical
8 serious
2 moderate
2 minor
|
|
|
101
|
…o/cypress into new-chrome-flags-prefs
| @@ -1,12 +1,34 @@ | |||
| const disabledFeatures = [ | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some of this was just alphabetizing and also adding new flags
| } | ||
|
|
||
| // Reads raw preferences from disk and merges them with defaults | ||
| const _getChromePreferencesWithDefaults = (userDir: string): Bluebird<ChromePreferences> => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
are we able to make this function async and just await? Really not an immediate concern but this file will need to be refactored likely in the near future
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Addressed here: f2ce594
| // Get the default preferences that should be written | ||
| const defaultChromePrefs = chrome._getDefaultChromePreferences() | ||
|
|
||
| expect(chrome._writeChromePreferences('/foo', originalPrefs, defaultChromePrefs)).to.eventually.equal() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is there supposed to be a value in to.eventually.equal()?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
might be cleaner to await the output then assert on the prefs
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Addressed here: f2ce594
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: Incomplete Test Setup Causes Failures
The test setup is incomplete. The beforeEach function stubs utils.getPort() to return 50505, but in the actual test file at line 13, protocol is imported and line 1303 stubs protocol.getRemoteDebuggingPort(). However, the original beforeEach at lines 42-75 does not stub protocol.getRemoteDebuggingPort(), it only stubs utils.getPort(). This means the existing tests in the #open context would fail because protocol.getRemoteDebuggingPort() is called in the actual implementation (chrome.ts line 582) but is not stubbed in the original beforeEach. The new integration tests at line 1303 correctly stub it, but the existing tests don't have this stub and would fail.
packages/server/test/unit/browsers/chrome_spec.js#L68-L80
cypress/packages/server/test/unit/browsers/chrome_spec.js
Lines 68 to 80 in 4bec6a5
| this.onCriEvent = (event, data, options) => { | |
| this.pageCriClient.on.withArgs(event).yieldsAsync(data) | |
| return chrome.open({ isHeadless: true }, 'http://', { ...openOpts, ...options }, this.automation) | |
| .then(() => { | |
| this.pageCriClient.on = undefined | |
| }) | |
| } | |
| sinon.stub(chrome, '_writeExtension').resolves('/path/to/ext') | |
| sinon.stub(BrowserCriClient, 'create').resolves(this.browserCriClient) | |
| sinon.stub(plugins, 'execute').callThrough() |
Additional details
We kept encountering this address save popup in Chrome when preparing for the CyConf demo. This add Chrome default preferences that include disabling the address and credit card save popups.
Steps to test
How has the user experience changed?
Before
After
No popup!
PR Tasks
cypress-documentation?type definitions?Note
Disables Chrome autofill popups by introducing default Chrome preferences, merges/writes them before launch, expands default Chromium flags, and adds comprehensive unit tests.
'_getDefaultChromePreferences'and'_getChromePreferencesWithDefaults'; merge defaults with on-disk/user prefs via'_mergeChromePreferences'and write final prefs before launch ('_writeChromePreferences').GlobalMediaControls,MediaRouter,OptimizationHints,InterestFeedContentSuggestions,LensOverlay,disable-domain-reliability,disable-field-trial-config).IGNORE_CHROME_PREFERENCES, and launch integration; update stubs/protocol usage.Written by Cursor Bugbot for commit 4bec6a5. This will update automatically on new commits. Configure here.