Skip to content

[rb] Update unhandled_prompt_behavior capability to support hash syntax.#16289

Merged
p0deje merged 6 commits intoSeleniumHQ:trunkfrom
Osseta:fix-ruby-handling-of-unhandled_prompt_behavior
Sep 9, 2025
Merged

[rb] Update unhandled_prompt_behavior capability to support hash syntax.#16289
p0deje merged 6 commits intoSeleniumHQ:trunkfrom
Osseta:fix-ruby-handling-of-unhandled_prompt_behavior

Conversation

@Osseta
Copy link
Copy Markdown
Contributor

@Osseta Osseta commented Sep 4, 2025 •
edited by qodo-code-review bot
Loading

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
Copy Markdown

CLAassistant commented Sep 4, 2025 •
edited
Loading

CLA assistant check
All committers have signed the CLA.

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

qodo-code-review 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

@Osseta Osseta mentioned this pull request Sep 4, 2025
Closed
@qodo-code-review
Copy link
Copy Markdown
Contributor

qodo-code-review bot commented Sep 4, 2025 •
edited
Loading

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
p0deje
p0deje approved these changes Sep 8, 2025
View reviewed changes
Copy link
Copy Markdown
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
Copy Markdown
Member

p0deje commented Sep 9, 2025

Thank you for your contribution!

@aguspe aguspe mentioned this pull request Dec 14, 2025
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@p0deje p0deje p0deje approved these changes

Assignees

No one assigned

Labels

C-rb Ruby Bindings Review effort 2/5

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@Osseta @CLAassistant @p0deje @selenium-ci @aguspe