Skip to content

Conversation

Osseta
Copy link
Contributor

@Osseta Osseta commented Sep 4, 2025

User description

💥 What does this PR do?

This PR extends the capability processing method to correctly support the struct format of UserPromptHandler for unhandledPromptBehaviour in a CapabilityRequest

The current implementation supports the "string" only values format. For correct BiDi operation the more complex hash format is required due to a recent change in the Chrome BiDi driver code to enforce beforeUnload to be accept when only strings are used.

The code that worked before the Chrome BiDi update:

Selenium::WebDriver::Chrome.new(unhandled_prompt_behavior: :ignore)

The code that now does the same after the Chrome BiDi update:

Selenium::WebDriver::Chrome.new(unhandled_prompt_behavior: {
  before_unload: :ignore,
  default: :ignore
})

🔧 Implementation Notes

The implement is backwards compatible with existing code .

💡 Additional Considerations

This PR is to resolve an issue I originally raised in the Capybara project teamcapybara/capybara#2824

This PR does not update the web documentation for Selenium

🔄 Types of changes

  • New feature (non-breaking change which adds functionality and tests!)

PR Type

Enhancement


Description

  • Add support for hash format in unhandled_prompt_behavior capability

  • Maintain backward compatibility with existing string format

  • Enable proper BiDi operation with Chrome driver updates

  • Add comprehensive test coverage for hash value processing


Diagram Walkthrough

flowchart LR
  A["String Format"] --> B["process_unhandled_prompt_behavior_value"]
  C["Hash Format"] --> B
  B --> D["Transformed Values"]
  D --> E["W3C Compatible Output"]
Loading

File Walkthrough

Relevant files
Enhancement
options.rb
Enhanced capability processing for hash format                     

rb/lib/selenium/webdriver/common/options.rb

  • Extract unhandled_prompt_behavior processing into dedicated method
  • Add recursive hash value transformation support
  • Maintain backward compatibility with string format
+9/-1     
Tests
options_spec.rb
Test coverage for hash format capability                                 

rb/spec/unit/selenium/webdriver/chrome/options_spec.rb

  • Add comprehensive test for hash format unhandled_prompt_behavior
  • Verify proper key transformation and value processing
  • Test multiple prompt types with different values
+20/-0   

@CLAassistant
Copy link

CLAassistant commented Sep 4, 2025

CLA assistant check
All committers have signed the CLA.

@selenium-ci selenium-ci added the C-rb Ruby Bindings label Sep 4, 2025
Copy link
Contributor

qodo-merge-pro bot commented Sep 4, 2025

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

🎫 Ticket compliance analysis 🔶

2824 - Partially compliant

Compliant requirements:

  • Ensure compatibility with Chrome/Chromedriver behavior changes.

Non-compliant requirements:

  • Address issues with Selenium WebDriver waiting behavior on macOS Sierra where expected-condition waits don't retry and instead immediately raise NoSuchElementException.
  • Provide a fix or workaround enabling proper waiting/retry semantics.

Requires further human verification:

  • Verify in real environments (macOS Sierra or representative setups) that waits behave correctly; current PR targets capability handling rather than wait logic.
⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
🧪 PR contains tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

Type Handling

When processing unhandled_prompt_behavior, symbols, strings, and nested hashes are handled, but arrays are not. Confirm that BiDi won't ever provide arrays and that nil propagation in nested values is desired.

def process_unhandled_prompt_behavior_value(value)
  if value.is_a?(Hash)
    value.transform_values { |v| process_unhandled_prompt_behavior_value(v) }
  else
    value&.to_s&.tr('_', ' ')
  end
end
Mutability

process_w3c_options deletes keys from the original options hash; verify no callers rely on the original unmodified input, especially with nested hash for unhandled_prompt_behavior.

def process_w3c_options(options)
  w3c_options = options.select { |key, val| w3c?(key) && !val.nil? }
  w3c_options[:unhandled_prompt_behavior] &&= process_unhandled_prompt_behavior_value(w3c_options[:unhandled_prompt_behavior])
  options.delete_if { |key, _val| w3c?(key) }
  w3c_options
end

Copy link
Contributor

qodo-merge-pro bot commented Sep 4, 2025

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
Possible issue
Camelize nested option keys

Also transform hash keys to the expected W3C/camelCase format (e.g.,
before_unload -> beforeUnload) when handling nested values. Without this,
drivers may ignore options like beforeUnload and the new spec test will fail.
Convert keys and recurse on values.

rb/lib/selenium/webdriver/common/options.rb [139-145]

 def process_unhandled_prompt_behavior_value(value)
-  if value.is_a?(Hash)
-    value.transform_values { |v| process_unhandled_prompt_behavior_value(v) }
+  case value
+  when Hash
+    value
+      .transform_keys { |k| k.to_s.gsub(/_([a-z])/) { Regexp.last_match(1).upcase } }
+      .transform_values { |v| process_unhandled_prompt_behavior_value(v) }
   else
     value&.to_s&.tr('_', ' ')
   end
 end
  • Apply / Chat
Suggestion importance[1-10]: 9

__

Why: The suggestion correctly identifies that the keys of the unhandled_prompt_behavior hash are not being converted to camelCase, which is required by the W3C standard and the new test case. Without this change, the new functionality would be broken.

High
  • Update

@Osseta Osseta force-pushed the fix-ruby-handling-of-unhandled_prompt_behavior branch from 6d33076 to a6558cc Compare September 7, 2025 22:13
@Osseta Osseta changed the title Update unhandled_prompt_behavior capability to support hash syntax. [rb] Update unhandled_prompt_behavior capability to support hash syntax. Sep 8, 2025
Copy link
Member

@p0deje p0deje left a comment

Choose a reason for hiding this comment

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

The changes look great. Can you please just fix a formatting issue (probably a trailing space), and then it's good to merge.

@p0deje p0deje merged commit f5ad7eb into SeleniumHQ:trunk Sep 9, 2025
22 checks passed
@p0deje
Copy link
Member

p0deje commented Sep 9, 2025

Thank you for your contribution!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants