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

def closeBrowser(self) use .quit() instead of .close() #125

Closed
wants to merge 3 commits into from
Closed

def closeBrowser(self) use .quit() instead of .close() #125

wants to merge 3 commits into from

Conversation

jdeath
Copy link

@jdeath jdeath commented Jun 19, 2024

Using .close() in def closeBrowser(self) caused a crash when restarting the session in mobile browser mode in my docker container. Changing to .quit(), which is also used in def getChromeVersion(self) allowed the mobile mode to start out. I tested in non-docker and it did not cause an issue

@cal4
Copy link
Collaborator

cal4 commented Jun 19, 2024

For me, if I quit rather than close, there's orphaned Chrome processes

@jdeath
Copy link
Author

jdeath commented Jun 19, 2024

Darn! I did not check that. But in the docker it definitely crashes. I think I tried .close() in both closeBrowser and getChromeVersion and that still crashed. I'll double check. I might just need to keep a patch going for my use case, which is less than ideal. This thing is so cooll!!

@cal4
Copy link
Collaborator

cal4 commented Jun 19, 2024

Yeah, I could be wrong so definitely worth a double-check.

From what I can tell, undetected_chromedriver's Chrome overrides the parents quit and exit, while close isn't.

See ultrafunkamsterdam/undetected-chromedriver#1708, ultrafunkamsterdam/undetected-chromedriver#1667 and ultrafunkamsterdam/undetected-chromedriver#1507 (probably more).

@jdeath
Copy link
Author

jdeath commented Jun 19, 2024

ok. Someone else just posted a PR with a close, then quit and removed some extra closes. I will try that when my day resets.

edit: close then quit caused a crash.

@jdeath
Copy link
Author

jdeath commented Jun 22, 2024

Closing. I can patch my instance to use .quit() versus .close()

@jdeath jdeath closed this Jun 22, 2024
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