Skip to content

Conversation

@anilabhadatta
Copy link

Explained in #2602

@mdmintz
Copy link
Member

mdmintz commented Mar 16, 2024

Already resolved in 4.24.9 - https://github.com/seleniumbase/SeleniumBase/releases/tag/v4.24.9

@mdmintz mdmintz closed this Mar 16, 2024
@anilabhadatta
Copy link
Author

@mdmintz Not working
image

Line 240 243, these lines execute correctly but in Windows it will open the chrome browser instead of getting the version info via terminal.
The browser is opened two times.
Please give a check that if it is windows try to get the version using Powershell, If failed then try to get the version using the commented lines.

@mdmintz
Copy link
Member

mdmintz commented Mar 16, 2024

That method is working for me in Windows on version 4.24.9:

import os
from seleniumbase.core import detect_b_ver

path = "C:\\Program Files (x86)\\Google\\Chrome\\Application\\chrome.exe"
print(os.path.exists(path))
print(detect_b_ver.get_browser_version_from_binary(path))

Output:

True
122.0.6261.112

If something isn't working for you, you need to provide a minimal reproducible example, with clear steps showing the code that you used to see the outcome that you had.

@anilabhadatta
Copy link
Author

anilabhadatta commented Mar 16, 2024

@mdmintz
image
image

Whenever a system path's chrome is being used, it is working
But if a custom chrome path is provided then it is not working
It maybe like this that the custom chrome path is of chrome for testing which may not support the method used to get chrome version.
If I try to use the method, then a chrome browser pops up twice and then prints the version.
I am creating a PR, which will resolve the issue.
It will check if the OS is windows and try to get the chrome version using the powershell cmd, else use --version.
This should work for any OS I believe

@anilabhadatta anilabhadatta mentioned this pull request Mar 16, 2024
@mdmintz
Copy link
Member

mdmintz commented Mar 16, 2024

I copied chrome.exe to C:\\tmp\\chrome.exe, (which isn't on the System PATH), and it works correctly on 4.24.9:

import os
from seleniumbase.core import detect_b_ver

path = "C:\\tmp\\chrome.exe"
print(os.path.exists(path))
print(detect_b_ver.get_browser_version_from_binary(path))

Output:

True
122.0.6261.112

@anilabhadatta
Copy link
Author

@mdmintz I think this is a chrome for testing only issue.
I have tested now, read_version_from_cmd()
This method returns None when --version flag is used initially
But in Chrome for testing it will first open the browser and then send None while closing the browser.
So I think it is better to use the powershell command to get the version from Windows Chrome, otherwise use --version flag if it fails.

@mdmintz
Copy link
Member

mdmintz commented Mar 16, 2024

I also tried with a chrome-for-testing Chrome, and there was no extra browser that spun up for me. SeleniumBase is also for regular Chrome browsers (not chrome-for-testing ones, as that won't work in UC Mode).

@anilabhadatta
Copy link
Author

@mdmintz seleniumbase does work for chrome for testing, cloudflare turnstile is bypassed as well

@mdmintz
Copy link
Member

mdmintz commented Mar 16, 2024

That's good to hear. I still cannot reproduce your issue with the extra browser. Stop creating multiple pull requests for it.

@anilabhadatta
Copy link
Author

image

@mdmintz
Copy link
Member

mdmintz commented Mar 16, 2024

That's good that it works. The PRs are still unnecessary.

@anilabhadatta
Copy link
Author

@mdmintz I will write a simple Python script, do check out if that works on your Windows machine.
I have just tested it on my Windows 10 machine as well, I was able to recreate the scenario.

@anilabhadatta
Copy link
Author

anilabhadatta commented Mar 16, 2024

@mdmintz Install the necessary dependencies if required, Run this code and send me the output, please.
As per my testing, if a relative path is used then your method read_version_from_cmd() on line 240 and 243 will return None without any browser pop up, But an absolute path will automatically create a browser instance in Windows.

import zipfile
from seleniumbase.core import detect_b_ver
import wget
import os

wget.download("https://storage.googleapis.com/chrome-for-testing-public/116.0.5845.96/win64/chrome-win64.zip", 
              out=".")
print("Download Complete now Extracting...")
with zipfile.ZipFile("chrome-win64.zip", "r") as zip_ref:
    zip_ref.extractall(".")
    print("Download and Extraction of Chrome completed.")
start_time = datetime.now()
chrome_path = os.path.join(os.getcwd(), "chrome-win64", "chrome.exe")
print(detect_b_ver.get_browser_version_from_binary(chrome_path))
end_time = datetime.now()
print('Duration: {}'.format(end_time - start_time))```

@mdmintz
Copy link
Member

mdmintz commented Mar 16, 2024

Thank you. That was the real issue: Getting the Chrome-for-Testing version on Windows from the binary_location, which was not related to absolute or relative paths.

Fixed in 4.24.10 - https://github.com/seleniumbase/SeleniumBase/releases/tag/v4.24.10

Also, the powershell Get-Command was faster than Get-Item by 0.05 seconds in testing.

@anilabhadatta
Copy link
Author

@mdmintz Thanks for the latest commit, should be fixed now.
However you could refer to my last commit where I added a Exception handling when powershell might not be present like Win 7 or older.
And in my system when I tested using ./chrome-x64/chrome.exe, the method previously worked fine but using the absolute path opened up the browser instance.

@mdmintz
Copy link
Member

mdmintz commented Mar 16, 2024

Relative paths are working fine for me if the path exists.

Also, not going to bother with maintaining support on Windows 7 or older for this specific case. If people really need to use Windows 7, they can put the browser on their system path.

If the path doesn't exist on Windows, then the method will return None without opening any browsers.

@mdmintz
Copy link
Member

mdmintz commented Mar 16, 2024

@anilabhadatta
Copy link
Author

@mdmintz thanks will check and test the latest commit

@anilabhadatta
Copy link
Author

@mdmintz Working fine.

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