From ffca0f04cbd2253bfc9499bee561c9959c08c696 Mon Sep 17 00:00:00 2001 From: Sean Doyle Date: Thu, 23 Jul 2026 14:38:31 -0400 Subject: [PATCH 1/2] Introduce `raise_on_unhandled_modal` browser configuration MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The problem --- When executing a system test suite that relies on `confirm`, `prompt`, or other browser-level modals, the default behavior to ignore modally presented dialogs can cause false negatives. For example, a change to the implementation might accidentally introduce a perpetually prompting confirmation modal. While the test suite outputs "Modal window … has been opened" warnings, the underlying test still passes. The proposal --- This commit proposes a new Cuprite-level `:raise_on_unhandled_modal` option to control whether an unhandled modal warns, or raises. When set to `true`, then false negative test would fail, rather than pass. --- README.md | 1 + lib/capybara/cuprite/browser.rb | 9 +++++++++ lib/capybara/cuprite/driver.rb | 1 + lib/capybara/cuprite/options.rb | 2 +- lib/capybara/cuprite/page.rb | 6 +++++- spec/features/session_spec.rb | 12 ++++++++++++ 6 files changed, 29 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 91854cf2..8c4a3496 100644 --- a/README.md +++ b/README.md @@ -76,6 +76,7 @@ end `Cuprite`-specific options are: * options `Hash` + * `:raise_on_unhandled_modal` (Boolean) - When set to `false`, output a warning. When set to `true`, raise an exception * `:url_blacklist` (Array) - array of regexes to match against requested URLs * `:url_whitelist` (Array) - array of regexes to match against requested URLs diff --git a/lib/capybara/cuprite/browser.rb b/lib/capybara/cuprite/browser.rb index 8ffa254c..d90761bb 100644 --- a/lib/capybara/cuprite/browser.rb +++ b/lib/capybara/cuprite/browser.rb @@ -16,6 +16,7 @@ class Browser < Ferrum::Browser def initialize(options = nil) super + @options.raise_on_unhandled_modal = options&.delete(:raise_on_unhandled_modal) @options.url_blacklist = prepare_wildcards(options&.dig(:url_blacklist)) @options.url_whitelist = prepare_wildcards(options&.dig(:url_whitelist)) @@ -51,6 +52,14 @@ def resize(**options) super end + def raise_on_unhandled_modal + @options.raise_on_unhandled_modal + end + + def raise_on_unhandled_modal=(value) + @options.raise_on_unhandled_modal = value + end + def url_whitelist @options.url_whitelist end diff --git a/lib/capybara/cuprite/driver.rb b/lib/capybara/cuprite/driver.rb index 0eec6ba8..2e31d4c2 100644 --- a/lib/capybara/cuprite/driver.rb +++ b/lib/capybara/cuprite/driver.rb @@ -137,6 +137,7 @@ def reset! @paper_size = nil browser.url_blacklist = @options[:url_blacklist] browser.url_whitelist = @options[:url_whitelist] + browser.raise_on_unhandled_modal = @options.fetch(:raise_on_unhandled_modal, false) browser.reset @started = false end diff --git a/lib/capybara/cuprite/options.rb b/lib/capybara/cuprite/options.rb index 1f863dee..40a124c7 100644 --- a/lib/capybara/cuprite/options.rb +++ b/lib/capybara/cuprite/options.rb @@ -4,7 +4,7 @@ module Ferrum class Browser class Options attr_writer :window_size - attr_accessor :url_blacklist, :url_whitelist + attr_accessor :url_blacklist, :url_whitelist, :raise_on_unhandled_modal def reset_window_size @window_size = @options[:window_size] diff --git a/lib/capybara/cuprite/page.rb b/lib/capybara/cuprite/page.rb index 2b21a863..2fefc9c6 100644 --- a/lib/capybara/cuprite/page.rb +++ b/lib/capybara/cuprite/page.rb @@ -162,10 +162,14 @@ def prepare_page response = @modal_response || params["defaultPrompt"] else with_text = params["message"] ? "with text `#{params['message']}` " : "" - warn "Modal window #{with_text}has been opened, but you didn't wrap " \ + message = + "Modal window #{with_text}has been opened, but you didn't wrap " \ "your code into (`accept_prompt` | `dismiss_prompt` | " \ "`accept_confirm` | `dismiss_confirm` | `accept_alert`), " \ "accepting by default" + + @options.raise_on_unhandled_modal ? raise(message) : warn(message) + options = { accept: true } response = params["defaultPrompt"] end diff --git a/spec/features/session_spec.rb b/spec/features/session_spec.rb index 209c3261..33225bc2 100644 --- a/spec/features/session_spec.rb +++ b/spec/features/session_spec.rb @@ -1168,6 +1168,18 @@ expect(@session).to have_xpath("//a[@id='open-match' and @confirmed='true']") end + it "configured to raise warning" do + @session.driver.browser.raise_on_unhandled_modal = true + + @session.visit "/cuprite/with_js" + + expect { @session.click_link("Open for match") }.to raise_error( + "Modal window with text `{T}ext \\w|th [reg.exp] (chara©+er$)?` has been opened, " \ + "but you didn't wrap your code into (`accept_prompt` | `dismiss_prompt` | `accept_confirm` " \ + "| `dismiss_confirm` | `accept_alert`), accepting by default" + ) + end + it "matches on partial strings" do @session.visit "/cuprite/with_js" expect do From e6751bb68d4a4c6da719e545b1b4513310aee2ba Mon Sep 17 00:00:00 2001 From: Dmitry Vorotilin Date: Tue, 25 Aug 2026 03:56:40 +0300 Subject: [PATCH 2/2] Fix raise_on_unhandled_modal option and make raising safe MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Browser#initialize mutated the caller's options hash with delete instead of dig, so Driver#reset! (called between every Capybara example) always saw the option as absent and forced it back to false after the very first use — the option never actually took effect via normal driver configuration. Raising directly from the Page.javascriptDialogOpening handler also doesn't work: that callback runs on Ferrum's background CDP dispatcher thread, so the exception never reaches the caller and permanently kills that thread, wedging the browser for the rest of the suite (reproduced: it cascades into unrelated failures in later examples). The dialog is now always answered first, on Ferrum's own command (captured via alias before overriding it), and the error is deferred and re-raised from the main thread on its next command call via ensure, so it fires whether or not that command itself raised. Also raises a proper Capybara::Cuprite::UnhandledModalError instead of a bare RuntimeError, and adds a CHANGELOG entry. --- CHANGELOG.md | 1 + README.md | 7 ++++- lib/capybara/cuprite/browser.rb | 2 +- lib/capybara/cuprite/errors.rb | 9 ++++++ lib/capybara/cuprite/page.rb | 50 ++++++++++++++++++++++++++++----- spec/features/session_spec.rb | 3 +- spec/lib/driver_spec.rb | 11 ++++++++ 7 files changed, 73 insertions(+), 10 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9a09adb3..c8d775c5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,7 @@ - Extend `Node#set` with the `datetime-local` input type [#295] - Support HTML5 drag-and-drop and drag modifier keys in `Node#drag_to` [#315] - Support `Element#drop` for files and strings [#316] +- Add `:raise_on_unhandled_modal` browser option to raise instead of warn on an unhandled modal [#320] ### Changed - Bump Ferrum dependency to `~> 0.18.0` diff --git a/README.md b/README.md index 8c4a3496..5b906a41 100644 --- a/README.md +++ b/README.md @@ -76,7 +76,12 @@ end `Cuprite`-specific options are: * options `Hash` - * `:raise_on_unhandled_modal` (Boolean) - When set to `false`, output a warning. When set to `true`, raise an exception + * `:raise_on_unhandled_modal` (Boolean) - When set to `false`, output a warning. When set to `true`, raise + `Capybara::Cuprite::UnhandledModalError` instead. The dialog is always auto-accepted either way; the raise is + deferred and surfaces on the next command sent to the browser. In practice that's almost always the same action + that triggered the dialog (a click, `visit`, `evaluate_script`, ...), since a JS dialog blocks the page until + answered, so that action's own command is what was waiting. A dialog fired with nothing in flight (e.g. a bare JS + timer) only surfaces on whatever command runs next. * `:url_blacklist` (Array) - array of regexes to match against requested URLs * `:url_whitelist` (Array) - array of regexes to match against requested URLs diff --git a/lib/capybara/cuprite/browser.rb b/lib/capybara/cuprite/browser.rb index d90761bb..5b684484 100644 --- a/lib/capybara/cuprite/browser.rb +++ b/lib/capybara/cuprite/browser.rb @@ -16,7 +16,7 @@ class Browser < Ferrum::Browser def initialize(options = nil) super - @options.raise_on_unhandled_modal = options&.delete(:raise_on_unhandled_modal) + @options.raise_on_unhandled_modal = options&.dig(:raise_on_unhandled_modal) @options.url_blacklist = prepare_wildcards(options&.dig(:url_blacklist)) @options.url_whitelist = prepare_wildcards(options&.dig(:url_whitelist)) diff --git a/lib/capybara/cuprite/errors.rb b/lib/capybara/cuprite/errors.rb index 0b176143..61c0843b 100644 --- a/lib/capybara/cuprite/errors.rb +++ b/lib/capybara/cuprite/errors.rb @@ -43,6 +43,15 @@ def message end end + class UnhandledModalError < Error + attr_reader :message + + def initialize(message) + @message = message + super() + end + end + class ObsoleteNode < ClientError attr_reader :node diff --git a/lib/capybara/cuprite/page.rb b/lib/capybara/cuprite/page.rb index 2fefc9c6..fa342519 100644 --- a/lib/capybara/cuprite/page.rb +++ b/lib/capybara/cuprite/page.rb @@ -25,9 +25,25 @@ def initialize(...) @accept_modal = [] @modal_messages = [] @modal_response = nil + @unhandled_modal_error = nil super end + # Keep a handle to Ferrum's own implementation before overriding it + # below, so answering a dialog (see `handle_javascript_dialog`) can + # bypass our override. + alias ferrum_command command + + # The `Page.javascriptDialogOpening` event is handled on Ferrum's + # background CDP dispatcher thread, so raising there wouldn't reach + # the caller and would permanently kill that thread instead. The + # dialog is always accepted immediately from that thread, and the + # error (if any) is stashed here to be raised from the main thread + # the next time it makes a command round trip. + def command(...) + raise_pending_unhandled_modal_error! { super } + end + def set(node, value) object_id = command("DOM.resolveNode", nodeId: node.node_id).dig("object", "objectId") evaluate("_cuprite.set(arguments[0], arguments[1])", { "objectId" => object_id }, value) @@ -100,6 +116,7 @@ def reset_modals @accept_modal = [] @modal_response = nil @modal_messages = [] + @unhandled_modal_error = nil end def before_click(node, name, _keys = [], offset = {}) @@ -141,6 +158,18 @@ def closed? private + def raise_pending_unhandled_modal_error! + error = @unhandled_modal_error + @unhandled_modal_error = nil + raise error if error + + yield + ensure + error = @unhandled_modal_error + @unhandled_modal_error = nil + raise error if error + end + def prepare_page super @@ -155,6 +184,8 @@ def prepare_page on("Page.javascriptDialogOpening") do |params| accept_modal = @accept_modal.last + unhandled_modal_error = nil + if [true, false].include?(accept_modal) @accept_modal.pop @modal_messages << params["message"] @@ -162,19 +193,24 @@ def prepare_page response = @modal_response || params["defaultPrompt"] else with_text = params["message"] ? "with text `#{params['message']}` " : "" - message = - "Modal window #{with_text}has been opened, but you didn't wrap " \ - "your code into (`accept_prompt` | `dismiss_prompt` | " \ - "`accept_confirm` | `dismiss_confirm` | `accept_alert`), " \ - "accepting by default" + message = "Modal window #{with_text}has been opened, but you didn't wrap " \ + "your code into (`accept_prompt` | `dismiss_prompt` | " \ + "`accept_confirm` | `dismiss_confirm` | `accept_alert`), " \ + "accepting by default" - @options.raise_on_unhandled_modal ? raise(message) : warn(message) + if @options.raise_on_unhandled_modal + unhandled_modal_error = UnhandledModalError.new(message) + else + warn message + end options = { accept: true } response = params["defaultPrompt"] end options.merge!(promptText: response) if response - command("Page.handleJavaScriptDialog", **options) + ferrum_command("Page.handleJavaScriptDialog", **options) + + @unhandled_modal_error = unhandled_modal_error if unhandled_modal_error end end diff --git a/spec/features/session_spec.rb b/spec/features/session_spec.rb index 33225bc2..4ce33090 100644 --- a/spec/features/session_spec.rb +++ b/spec/features/session_spec.rb @@ -1168,12 +1168,13 @@ expect(@session).to have_xpath("//a[@id='open-match' and @confirmed='true']") end - it "configured to raise warning" do + it "configured to raise instead of warning" do @session.driver.browser.raise_on_unhandled_modal = true @session.visit "/cuprite/with_js" expect { @session.click_link("Open for match") }.to raise_error( + Capybara::Cuprite::UnhandledModalError, "Modal window with text `{T}ext \\w|th [reg.exp] (chara©+er$)?` has been opened, " \ "but you didn't wrap your code into (`accept_prompt` | `dismiss_prompt` | `accept_confirm` " \ "| `dismiss_confirm` | `accept_alert`), accepting by default" diff --git a/spec/lib/driver_spec.rb b/spec/lib/driver_spec.rb index 1ad6292f..fc07e5ad 100644 --- a/spec/lib/driver_spec.rb +++ b/spec/lib/driver_spec.rb @@ -9,6 +9,17 @@ end end + describe "raise_on_unhandled_modal configuration" do + it "survives resetting the driver between examples" do + driver = described_class.new(nil, { raise_on_unhandled_modal: true }) + + driver.browser + + expect { driver.reset! }.not_to(change { driver.browser.raise_on_unhandled_modal }) + expect(driver.browser.raise_on_unhandled_modal).to eq(true) + end + end + describe "save_path configuration" do it "defaults to the Capybara save path" do driver = with_capybara_save_path("/tmp/capybara-save-path") do