[rb] allow Options#add_option to set top level options if they use w3…

…c compliant namespace
SeleniumHQ · Sep 15, 2021 · a1c9131 · a1c9131
1 parent 47e47d9
commit a1c9131
Show file tree

Hide file tree

Showing 7 changed files with 66 additions and 19 deletions.
diff --git a/rb/lib/selenium/webdriver/common/options.rb b/rb/lib/selenium/webdriver/common/options.rb
@@ -123,10 +123,14 @@ def as_json(*)
 
       private
 
+      def w3c?(key)
+        W3C_OPTIONS.include?(key) || key.to_s.include?(':')
+      end
+
       def process_w3c_options(options)
-        w3c_options = options.select { |key, _val| W3C_OPTIONS.include?(key) }
+        w3c_options = options.select { |key, _val| w3c?(key) }
         w3c_options[:unhandled_prompt_behavior] &&= w3c_options[:unhandled_prompt_behavior]&.to_s&.tr('_', ' ')
-        options.delete_if { |key, _val| W3C_OPTIONS.include?(key) }
+        options.delete_if { |key, _val| w3c?(key) }
         w3c_options
       end
 
@@ -173,6 +177,8 @@ def convert_json_key(key, camelize: true)
       def camel_case(str)
         str.gsub(/_([a-z])/) { Regexp.last_match(1).upcase }
       end
-    end # Options
+    end
+
+    # Options
   end # WebDriver
 end # Selenium
diff --git a/rb/lib/selenium/webdriver/safari/options.rb b/rb/lib/selenium/webdriver/safari/options.rb
@@ -28,6 +28,12 @@ class Options < WebDriver::Options
                         automatic_profiling: 'safari:automaticProfiling'}.freeze
         BROWSER = 'safari'
 
+        def add_option(name, value)
+          raise ArgumentError, 'Safari does not support options that are not namespaced' unless name.to_s.include?(':')
+
+          super
+        end
+
       end # Options
     end # Safari
   end # WebDriver

diff --git a/rb/spec/unit/selenium/webdriver/chrome/options_spec.rb b/rb/spec/unit/selenium/webdriver/chrome/options_spec.rb
@@ -212,9 +212,12 @@ module Chrome
             expect(options.as_json).to eq("browserName" => "chrome", "goog:chromeOptions" => {})
           end
 
-          it 'returns added option' do
+          it 'returns added options' do
             options.add_option(:foo, 'bar')
-            expect(options.as_json).to eq("browserName" => "chrome", "goog:chromeOptions" => {"foo" => "bar"})
+            options.add_option('foo:bar', {foo: 'bar'})
+            expect(options.as_json).to eq("browserName" => "chrome",
+                                          "foo:bar" => {"foo" => "bar"},
+                                          "goog:chromeOptions" => {"foo" => "bar"})
           end
 
           it 'converts profile' do
@@ -260,7 +263,8 @@ module Chrome
                                exclude_switches: %w[foobar barfoo],
                                minidump_path: 'linux/only',
                                perf_logging_prefs: {'enable_network': true},
-                               window_types: %w[normal devtools])
+                               window_types: %w[normal devtools],
+                               'custom:options': {foo: 'bar'})
 
             key = 'goog:chromeOptions'
             expect(opts.as_json).to eq('browserName' => 'chrome',
@@ -274,6 +278,7 @@ module Chrome
                                                       'pageLoad' => 400000,
                                                       'implicit' => 1},
                                        'setWindowRect' => false,
+                                       'custom:options' => {'foo' => 'bar'},
                                        key => {'args' => %w[foo bar],
                                                'prefs' => {'foo' => 'bar',
                                                            'key_that_should_not_be_camelcased' => 'baz',

diff --git a/rb/spec/unit/selenium/webdriver/edge/options_spec.rb b/rb/spec/unit/selenium/webdriver/edge/options_spec.rb
@@ -168,9 +168,12 @@ module Edge
             expect(options.as_json).to eq("browserName" => "MicrosoftEdge", "ms:edgeOptions" => {})
           end
 
-          it 'returns added option' do
+          it 'returns added options' do
             options.add_option(:foo, 'bar')
-            expect(options.as_json).to eq("browserName" => "MicrosoftEdge", "ms:edgeOptions" => {"foo" => "bar"})
+            options.add_option('foo:bar', {foo: 'bar'})
+            expect(options.as_json).to eq("browserName" => "MicrosoftEdge",
+                                          "foo:bar" => {"foo" => "bar"},
+                                          "ms:edgeOptions" => {"foo" => "bar"})
           end
 
           it 'returns a JSON hash' do
@@ -202,7 +205,8 @@ module Edge
                                exclude_switches: %w[foobar barfoo],
                                minidump_path: 'linux/only',
                                perf_logging_prefs: {'enable_network': true},
-                               window_types: %w[normal devtools])
+                               window_types: %w[normal devtools],
+                               'custom:options': {foo: 'bar'})
 
             key = 'ms:edgeOptions'
             expect(opts.as_json).to eq('browserName' => 'MicrosoftEdge',
@@ -216,6 +220,7 @@ module Edge
                                                       'pageLoad' => 400000,
                                                       'implicit' => 1},
                                        'setWindowRect' => false,
+                                       'custom:options' => {'foo' => 'bar'},
                                        key => {'args' => %w[foo bar],
                                                'prefs' => {'foo' => 'bar',
                                                            'key_that_should_not_be_camelcased' => 'baz'},

diff --git a/rb/spec/unit/selenium/webdriver/firefox/options_spec.rb b/rb/spec/unit/selenium/webdriver/firefox/options_spec.rb
@@ -143,9 +143,12 @@ module Firefox
             expect(options.as_json).to eq("browserName" => "firefox", "moz:firefoxOptions" => {})
           end
 
-          it 'returns added option' do
+          it 'returns added options' do
             options.add_option(:foo, 'bar')
-            expect(options.as_json).to eq("browserName" => "firefox", "moz:firefoxOptions" => {"foo" => "bar"})
+            options.add_option('foo:bar', {foo: 'bar'})
+            expect(options.as_json).to eq("browserName" => "firefox",
+                                          "foo:bar" => {"foo" => "bar"},
+                                          "moz:firefoxOptions" => {"foo" => "bar"})
           end
 
           it 'converts to a json hash' do
@@ -167,7 +170,8 @@ module Firefox
                                prefs: {foo: 'bar'},
                                foo: 'bar',
                                profile: profile,
-                               log_level: :debug)
+                               log_level: :debug,
+                               'custom:options': {foo: 'bar'})
 
             key = 'moz:firefoxOptions'
             expect(opts.as_json).to eq('browserName' => 'firefox',
@@ -181,6 +185,7 @@ module Firefox
                                                       'pageLoad' => 400000,
                                                       'implicit' => 1},
                                        'setWindowRect' => false,
+                                       'custom:options' => {'foo' => 'bar'},
                                        key => {'args' => %w[foo bar],
                                                'binary' => '/foo/bar',
                                                'prefs' => {'foo' => 'bar'},

diff --git a/rb/spec/unit/selenium/webdriver/ie/options_spec.rb b/rb/spec/unit/selenium/webdriver/ie/options_spec.rb
@@ -114,9 +114,11 @@ module IE
                                           "se:ieOptions" => {"nativeEvents" => true})
           end
 
-          it 'returns added option' do
+          it 'returns added options' do
             options.add_option(:foo, 'bar')
+            options.add_option('foo:bar', {foo: 'bar'})
             expect(options.as_json).to eq("browserName" => "internet explorer",
+                                          "foo:bar" => {"foo" => "bar"},
                                           "se:ieOptions" => {"nativeEvents" => true, "foo" => "bar"})
           end
 
@@ -148,7 +150,8 @@ module IE
                                use_per_process_proxy: true,
                                use_legacy_file_upload_dialog_handling: true,
                                attach_to_edge_chrome: true,
-                               edge_executable_path: '/path/to/edge')
+                               edge_executable_path: '/path/to/edge',
+                               'custom:options': {foo: 'bar'})
 
             key = 'se:ieOptions'
             expect(opts.as_json).to eq('browserName' => 'internet explorer',
@@ -162,6 +165,7 @@ module IE
                                                       'pageLoad' => 400000,
                                                       'implicit' => 1},
                                        'setWindowRect' => false,
+                                       'custom:options' => {'foo' => 'bar'},
                                        key => {'ie.browserCommandLineSwitches' => 'foo bar',
                                                'browserAttachTimeout' => 30000,
                                                'elementScrollBehavior' => 1,

diff --git a/rb/spec/unit/selenium/webdriver/safari/options_spec.rb b/rb/spec/unit/selenium/webdriver/safari/options_spec.rb
@@ -58,14 +58,28 @@ module Safari
           end
         end
 
+        describe '#add_option' do
+          it 'adds an option if name spaced' do
+            options.add_option('safari:foo', 'bar')
+            expect(options.instance_variable_get('@options')['safari:foo']).to eq('bar')
+          end
+
+          # Note that this only applies to the method, weird things can still happen
+          # if stuff is passed into the constructor
+          it 'raises exception if not name spaced' do
+            expect { options.add_option('foo', 'bar') }.to raise_exception(ArgumentError)
+          end
+        end
+
         describe '#as_json' do
           it 'returns empty options by default' do
             expect(options.as_json).to eq("browserName" => "safari")
           end
 
-          it 'returns added option' do
-            options.add_option(:foo, 'bar')
-            expect(options.as_json).to eq("browserName" => "safari", "foo" => "bar")
+          it 'returns added options' do
+            options.add_option('safari:foo', 'bar')
+            expect(options.as_json).to eq("browserName" => "safari",
+                                          "safari:foo" => "bar")
           end
 
           it 'returns JSON hash' do
@@ -80,7 +94,8 @@ module Safari
                                           implicit: 1},
                                set_window_rect: false,
                                automatic_profiling: false,
-                               automatic_inspection: true)
+                               automatic_inspection: true,
+                               'safari:foo': 'foo')
 
             expect(opts.as_json).to eq('browserName' => 'safari',
                                        'browserVersion' => '12',
@@ -94,7 +109,8 @@ module Safari
                                                       'implicit' => 1},
                                        'setWindowRect' => false,
                                        'safari:automaticInspection' => true,
-                                       'safari:automaticProfiling' => false)
+                                       'safari:automaticProfiling' => false,
+                                       'safari:foo' => 'foo')
           end
         end
       end # Options