SeleniumHQ / selenium

A browser automation framework and ecosystem.
https://selenium.dev
Apache License 2.0
30.58k stars 8.17k forks source link

[🚀 Feature]: Support passing Pathname objects for preference values in Ruby #11771

Closed md5 closed 1 year ago

md5 commented 1 year ago

Feature and motivation

After spending some time trying to track down why my download.default_directory preference for Chrome was not being honored, I determined that the issue was that I was passing a Pathname object instead of a String (via Rails.root.join). It would be great if the Ruby bindings honored Pathname objects by automatically calling #to_s on them. Barring that, it should at least error out instead of silently dropping the preference.

Usage example

options = Selenium::WebDriver::Chrome::Options.new
options.add_preference(
  :download,
  prompt_for_download: false,
  directory_upgrade: true,
  default_directory: Rails.root.join('tmp', 'capybara-downloads')
)

Currently, I have to pass Rails.root.join('tmp', 'capybara-downloads').to_s to get this to work.

github-actions[bot] commented 1 year ago

@md5, thank you for creating this issue. We will troubleshoot it as soon as we can.


Info for maintainers

Triage this issue by using labels.

If information is missing, add a helpful comment and then I-issue-template label.

If the issue is a question, add the I-question label.

If the issue is valid but there is no time to troubleshoot it, consider adding the help wanted label.

If the issue requires changes or fixes from an external project (e.g., ChromeDriver, GeckoDriver, MSEdgeDriver, W3C), add the applicable G-* label, and it will provide the correct link and auto-close the issue.

After troubleshooting the issue, please add the R-awaiting answer label.

Thank you!

titusfortner commented 1 year ago

So, the driver wants JSON, and Selenium is converting the provided Hash into JSON. Pathname is not a valid JSON data type. I guess we *could recursively parse everything and call #to_s for everything that isn't valid JSON type, but tbh this really should be responsibility of the user.

p0deje commented 1 year ago

Are you sure that it's Pathname used? I've just tried locally and it seems to convert to string correctly:

options = Selenium::WebDriver::Chrome::Options.new
options.add_preference(
  :download,
  prompt_for_download: false,
  directory_upgrade: true,
  default_directory: Pathname.new('/foo/bar')
)
Selenium::WebDriver.logger.level = :debug
driver = Selenium::WebDriver.for(:chrome, options: options)
...
2023-03-15 09:02:36 INFO Selenium -> POST session
2023-03-15 09:02:36 INFO Selenium    >>> http://127.0.0.1:9515/session | {"capabilities":{"alwaysMatch":{"browserName":"chrome","goog:chromeOptions":{"prefs":{"download":{"prompt_for_download":false,"directory_upgrade":true,"default_directory":"/foo/bar"}}}}}}
md5 commented 1 year ago

Are you sure that it's Pathname used? I've just tried locally and it seems to convert to string correctly

Perhaps not? I was definitely seeing default_directory omitted when I called options.to_json and it stopped being an issue when I called #to_s on the Pathname, but your test looks valid. The implementation of #as_json for Pathname also clearly just calls #to_s: https://github.com/rails/rails/blob/main/activesupport/lib/active_support/core_ext/object/json.rb#L230-L234

That code is unchanged since at least Rails 5.1.0... So I guess I'll close this issue as invalid. There was definitely something weird going on, but I'm not sure anymore what it was.

github-actions[bot] commented 10 months ago

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.