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(windows): Chrome folder is shown when no starting URL is provided #388

Merged
merged 3 commits into from
Dec 20, 2024

Conversation

going-confetti
Copy link
Collaborator

@going-confetti going-confetti commented Dec 19, 2024

Description

On Windows, if the user doesn't provide a URL before starting a recording, the browser shows the Chrome folder.
image

How to Test

On a windows machine:

  1. Open the app
  2. Go to Recorder
  3. Click Start recording while leaving the Starting URL blank
  4. Verify that the browser is launched with an empty page

@going-confetti going-confetti marked this pull request as ready for review December 19, 2024 09:21
@going-confetti going-confetti requested a review from a team as a code owner December 19, 2024 09:21
@going-confetti going-confetti requested review from cristianoventura, e-fisher and allansson and removed request for a team and cristianoventura December 19, 2024 09:21
src/browser.ts Outdated
@@ -70,7 +70,7 @@ export const launchBrowser = async (
`--proxy-server=http://localhost:${appSettings.proxy.port}`,
`--ignore-certificate-errors-spki-list=${certificateSPKI}`,
disableChromeOptimizations,
url ?? '',
url ?? 'about:blank',
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm still able to reproduce the issue on Windows. My guess url is an empty string and doesn't trigger nullish operator fallback, the || operator should do the trick.

You can also verify this on macos: about:blank should open as an empty page, currently it opens to chrome homepage with google search.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Might be worth to also to url.trim() to handle the user entering a bunch of spaces.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

You're right, I didn't even notice this on MacOS 🤦‍♂️

Copy link
Collaborator

@e-fisher e-fisher left a comment

Choose a reason for hiding this comment

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

🚀

@going-confetti going-confetti merged commit 7e6fd8a into main Dec 20, 2024
2 checks passed
@going-confetti going-confetti deleted the fix/windows-blank-page branch December 20, 2024 16:11
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