Feature/edge browser implementation #56
Conversation
|
|
||
| class Edge(Browser): | ||
| def __init__( | ||
| self, options: Options | None = None, connection_port: int = 9222 |
There was a problem hiding this comment.
Set the default port to None and use the Optional type, so the Base class will generate a random port. I modified the Chrome class since it was causing conflicts in multiple instances
| options.add_argument(f'--remote-debugging-port={connection_port}') | ||
| options.add_argument('--remote-allow-origins=*') | ||
|
|
||
| # Set default user data directory if not already set | ||
| if not any('--user-data-dir=' in arg for arg in options.arguments): | ||
| user_data_dir = os.path.join(os.path.expanduser('~'), '.edge_automation') | ||
| os.makedirs(user_data_dir, exist_ok=True) | ||
| options.add_argument(f'--user-data-dir={user_data_dir}') |
There was a problem hiding this comment.
When you call the super() method, these settings will be applied by default, so you don't need to duplicate them here
| options.add_argument('--no-first-run') # Disable first run experience | ||
| options.add_argument('--no-default-browser-check') # Disable default browser check | ||
| options.add_argument('--disable-crash-reporter') # Disable crash reporting | ||
| options.add_argument('--disable-features=TranslateUI') # Disable translation UI | ||
| options.add_argument('--disable-component-update') # Disable component updates | ||
| options.add_argument('--disable-background-networking') # Disable background networking |
There was a problem hiding this comment.
We have a class to manage the options called BrowserOptionsManager.
Here's my suggestion:
You can create an Enum with the available browsers, pass the browser name to the Browser class, and then forward it to the BrowserOptionsManager. There, you can create a method to configure the specific Edge options and call it if the specified browser is Edge.
Another idea: You can create a specific Options class that inherits from Options and set a property called browser. So, if the user wants to use Edge, they will have to use the specific Edge Options class.
In BrowserManagerOptions, you can check the browser property and then call the specific methods to configure each browser.
What do you think?
If you proceed with this idea, you'll have to create a specific class for Chrome too, inheriting from the base Options class and setting the browser property to "chrome".
|
Hi @Harris-H, First of all, thanks for your contribution! This feature is amazing. I just left some comments to maintain the Pydoll code style and ensure the separation of concerns. |
Description
Add Microsoft Edge browser automation support to pydoll library.
Changes
Testing
All tests passing, including Edge-specific test cases.
This PR addresses issue #41 by adding support for Microsoft Edge browser.