Repository: robotframework/SeleniumLibrary

Issue:
Default Capabilities not set correctly if remote_url and desired_capabilities are given
When using Open Browser keyword with remote_url and desired_capabilities then default capabilities are not set.
## Steps to reproduce the issue
Open browser using remote_url and desired_capabilities. For example test below.
```*** Settings ***
Library    SeleniumLibrary

*** Test Cases ***
Missing Capablities
    [Tags]   missing_caps
    SeleniumLibrary.Open Browser
    ...    url=https://www.google.fi
    ...    browser=chrome
    ...    remote_url=http://127.0.0.1:4444/wd/hub
    ...    desired_capabilities=build:10.1
```

## Error messages and additional information
When test is run with SeleniumLibrary 3.3.1 I can see the following lines in RF log file:
```
14:40:08.607	TRACE	Arguments: [ url='https://www.google.fi' | browser='chrome' | remote_url='http://127.0.0.1:4444/wd/hub' | desired_capabilities='build:10.1' ]	
14:40:08.607	INFO	Opening browser 'chrome' to base url 'https://www.google.fi' through remote server at 'http://127.0.0.1:4444/wd/hub'.	
14:40:09.609	DEBUG	POST http://127.0.0.1:4444/wd/hub/session {"capabilities": {"alwaysMatch": {}, "firstMatch": [{}]}, "desiredCapabilities": {"build": "10.1"}}
```
But when same test was run with SeleniumLibrary 3.2.0 the log output was as following
```
14:04:31.678	TRACE	Arguments: [ url='https://www.google.fi' | browser='chrome' | remote_url='http://127.0.0.1:4444/wd/hub' | desired_capabilities='build:10.1' ]	
14:04:31.678	INFO	Opening browser 'chrome' to base url 'https://www.google.fi' through remote server at 'http://127.0.0.1:4444/wd/hub'.	
14:04:32.679	DEBUG	POST http://127.0.0.1:4444/wd/hub/session {"capabilities": {"alwaysMatch": {"browserName": "chrome", "platformName": "any"}, "firstMatch": [{}]}, "desiredCapabilities": {"platform": "ANY", "browserName": "chrome", "version": "", "build": "10.1"}}
```

## Expected behavior and actual behavior

Expected: 
Default capabilities are set and and there is information for grid to know what browser to open.

Actual:
Now when the default capabilities are not set, grid fails to open browser since browserName is not set.

## Environment

Browser: Chrome 70.0.3538.67
Browser driver: Chromedriver
Operating System: Linux
Libraries
- Robot Framework: 3.1
- Selenium: 3.141.0
- SeleniumLibrary: 3.3.1 and 3.2.0
- Interpreter: Python 2.7.14


Discussion:

Patch:
diff --git a/src/SeleniumLibrary/keywords/webdrivertools.py b/src/SeleniumLibrary/keywords/webdrivertools.py
index 4bcd1afb..ff2b72f3 100644
--- a/src/SeleniumLibrary/keywords/webdrivertools.py
+++ b/src/SeleniumLibrary/keywords/webdrivertools.py
@@ -81,10 +81,21 @@ class WebDriverCreator(object):
             desired_capabilities[key.strip()] = value.strip()
         return desired_capabilities
 
+    def _remote_capabilities_resolver(self, set_capabilities, default_capabilities):
+        if not set_capabilities:
+            return {'desired_capabilities': default_capabilities}
+        if 'capabilities' in set_capabilities:
+            caps = set_capabilities['capabilities']
+        else:
+            caps = set_capabilities['desired_capabilities']
+        if 'browserName' not in caps:
+            caps['browserName'] = default_capabilities['browserName']
+        return {'desired_capabilities': caps}
+
     def create_chrome(self, desired_capabilities, remote_url, options=None):
         if is_truthy(remote_url):
-            if not desired_capabilities:
-                desired_capabilities = {'desired_capabilities': webdriver.DesiredCapabilities.CHROME.copy()}
+            defaul_caps = webdriver.DesiredCapabilities.CHROME.copy()
+            desired_capabilities = self._remote_capabilities_resolver(desired_capabilities, defaul_caps)
             return self._remote(desired_capabilities, remote_url, options=options)
         return webdriver.Chrome(options=options, **desired_capabilities)
 
@@ -98,8 +109,8 @@ class WebDriverCreator(object):
                        options=None):
         profile = self._get_ff_profile(ff_profile_dir)
         if is_truthy(remote_url):
-            if not desired_capabilities:
-                desired_capabilities = {'desired_capabilities': webdriver.DesiredCapabilities.FIREFOX.copy()}
+            defaul_caps = webdriver.DesiredCapabilities.FIREFOX.copy()
+            desired_capabilities = self._remote_capabilities_resolver(desired_capabilities, defaul_caps)
             return self._remote(desired_capabilities, remote_url,
                                 profile, options)
         desired_capabilities.update(self._geckodriver_log)
@@ -125,33 +136,29 @@ class WebDriverCreator(object):
 
     def create_ie(self, desired_capabilities, remote_url):
         if is_truthy(remote_url):
-            if not desired_capabilities:
-                ie = webdriver.DesiredCapabilities.INTERNETEXPLORER.copy()
-                desired_capabilities = {'desired_capabilities': ie}
+            defaul_caps = webdriver.DesiredCapabilities.INTERNETEXPLORER.copy()
+            desired_capabilities = self._remote_capabilities_resolver(desired_capabilities, defaul_caps)
             return self._remote(desired_capabilities, remote_url)
         return webdriver.Ie(**desired_capabilities)
 
     def create_edge(self, desired_capabilities, remote_url):
         if is_truthy(remote_url):
-            if not desired_capabilities:
-                edge = webdriver.DesiredCapabilities.EDGE.copy()
-                desired_capabilities = {'desired_capabilities': edge}
+            defaul_caps = webdriver.DesiredCapabilities.EDGE.copy()
+            desired_capabilities = self._remote_capabilities_resolver(desired_capabilities, defaul_caps)
             return self._remote(desired_capabilities, remote_url)
         return webdriver.Edge(**desired_capabilities)
 
     def create_opera(self, desired_capabilities, remote_url):
         if is_truthy(remote_url):
-            if not desired_capabilities:
-                opera = webdriver.DesiredCapabilities.OPERA.copy()
-                desired_capabilities = {'desired_capabilities': opera}
+            defaul_caps = webdriver.DesiredCapabilities.OPERA.copy()
+            desired_capabilities = self._remote_capabilities_resolver(desired_capabilities, defaul_caps)
             return self._remote(desired_capabilities, remote_url)
         return webdriver.Opera(**desired_capabilities)
 
     def create_safari(self, desired_capabilities, remote_url):
         if is_truthy(remote_url):
-            if not desired_capabilities:
-                caps = webdriver.DesiredCapabilities.SAFARI.copy()
-                desired_capabilities = {'desired_capabilities': caps}
+            defaul_caps = webdriver.DesiredCapabilities.SAFARI.copy()
+            desired_capabilities = self._remote_capabilities_resolver(desired_capabilities, defaul_caps)
             return self._remote(desired_capabilities, remote_url)
         return webdriver.Safari(**desired_capabilities)
 
@@ -159,38 +166,34 @@ class WebDriverCreator(object):
         warnings.warn('SeleniumLibrary support for PhantomJS has been deprecated, '
                       'please use headlesschrome or headlessfirefox instead.')
         if is_truthy(remote_url):
-            if not desired_capabilities:
-                caps = webdriver.DesiredCapabilities.PHANTOMJS.copy()
-                desired_capabilities = {'desired_capabilities': caps}
+            defaul_caps = webdriver.DesiredCapabilities.PHANTOMJS.copy()
+            desired_capabilities = self._remote_capabilities_resolver(desired_capabilities, defaul_caps)
             return self._remote(desired_capabilities, remote_url)
         return webdriver.PhantomJS(**desired_capabilities)
 
     def create_htmlunit(self, desired_capabilities, remote_url):
-        if not desired_capabilities:
-            desired_capabilities['desired_capabilities'] = webdriver.DesiredCapabilities.HTMLUNIT
+        defaul_caps = webdriver.DesiredCapabilities.HTMLUNIT.copy()
+        desired_capabilities = self._remote_capabilities_resolver(desired_capabilities, defaul_caps)
         return self._remote(desired_capabilities, remote_url)
 
     def create_htmlunit_with_js(self, desired_capabilities, remote_url):
-        if not desired_capabilities:
-            desired_capabilities['desired_capabilities'] = webdriver.DesiredCapabilities.HTMLUNITWITHJS
+        defaul_caps = webdriver.DesiredCapabilities.HTMLUNITWITHJS.copy()
+        desired_capabilities = self._remote_capabilities_resolver(desired_capabilities, defaul_caps)
         return self._remote(desired_capabilities, remote_url)
 
     def create_android(self, desired_capabilities, remote_url):
-        if not desired_capabilities:
-            desired_capabilities['desired_capabilities'] = webdriver.DesiredCapabilities.ANDROID
+        defaul_caps = webdriver.DesiredCapabilities.ANDROID.copy()
+        desired_capabilities = self._remote_capabilities_resolver(desired_capabilities, defaul_caps)
         return self._remote(desired_capabilities, remote_url)
 
     def create_iphone(self, desired_capabilities, remote_url):
-        if not desired_capabilities:
-            desired_capabilities['desired_capabilities'] = webdriver.DesiredCapabilities.IPHONE
+        defaul_caps = webdriver.DesiredCapabilities.IPHONE.copy()
+        desired_capabilities = self._remote_capabilities_resolver(desired_capabilities, defaul_caps)
         return self._remote(desired_capabilities, remote_url)
 
     def _remote(self, desired_capabilities, remote_url,
                 profile_dir=None, options=None):
         remote_url = str(remote_url)
-        if 'capabilities' in desired_capabilities:
-            desired_capabilities['desired_capabilities'] = desired_capabilities.pop('capabilities')
-
         return webdriver.Remote(command_executor=remote_url,
                                 browser_profile=profile_dir, options=options,
                                 **desired_capabilities)


Test patch:
diff --git a/utest/test/keywords/test_webdrivercreator.py b/utest/test/keywords/test_webdrivercreator.py
index 1996c97f..8c5cafa4 100644
--- a/utest/test/keywords/test_webdrivercreator.py
+++ b/utest/test/keywords/test_webdrivercreator.py
@@ -79,6 +79,33 @@ class WebDriverCreatorTests(unittest.TestCase):
             expected = {'capabilities': {'key1': 'value1', 'key2': 'value2'}}
             self.assertDictEqual(caps, expected)
 
+    def test_capabilities_resolver_firefox(self):
+        default_capabilities = webdriver.DesiredCapabilities.FIREFOX.copy()
+        expected_caps = {'desired_capabilities': {'version': '66.02', 'browserName': 'firefox'}}
+        caps_in = {'capabilities': {'version': '66.02'}}
+        resolved_caps = self.creator._remote_capabilities_resolver(caps_in, default_capabilities)
+        self.assertEqual(resolved_caps, expected_caps)
+
+        caps_in = {'capabilities': {'version': '66.02', 'browserName': 'firefox'}}
+        resolved_caps = self.creator._remote_capabilities_resolver(caps_in, default_capabilities)
+        self.assertEqual(resolved_caps, expected_caps)
+
+    def test_capabilities_resolver_no_set_caps(self):
+        default_capabilities = webdriver.DesiredCapabilities.FIREFOX.copy()
+        resolved_caps = self.creator._remote_capabilities_resolver({}, default_capabilities)
+        self.assertEqual(resolved_caps, {'desired_capabilities': default_capabilities})
+
+    def test_capabilities_resolver_chrome(self):
+        default_capabilities = webdriver.DesiredCapabilities.CHROME.copy()
+        expected_caps = {'desired_capabilities': {'version': '73.0.3683.86', 'browserName': 'chrome'}}
+        resolved_caps = self.creator._remote_capabilities_resolver({'capabilities': {'version': '73.0.3683.86'}},
+                                                                   default_capabilities)
+        self.assertEqual(resolved_caps, expected_caps)
+
+        caps_in = {'desired_capabilities': {'version': '73.0.3683.86', 'browserName': 'chrome'}}
+        resolved_caps = self.creator._remote_capabilities_resolver(caps_in, default_capabilities)
+        self.assertEqual(resolved_caps, expected_caps)
+
     def test_chrome(self):
         expected_webdriver = mock()
         when(webdriver).Chrome(options=None).thenReturn(expected_webdriver)
@@ -113,6 +140,17 @@ class WebDriverCreatorTests(unittest.TestCase):
         driver = self.creator.create_chrome({'desired_capabilities': capabilities}, url)
         self.assertEqual(driver, expected_webdriver)
 
+    def test_chrome_remote_caps_no_browser_name(self):
+        url = 'http://localhost:4444/wd/hub'
+        expected_webdriver = mock()
+        capabilities = {'browserName': 'chrome', 'key': 'value'}
+        when(webdriver).Remote(command_executor=url,
+                               browser_profile=None,
+                               desired_capabilities=capabilities,
+                               options=None).thenReturn(expected_webdriver)
+        driver = self.creator.create_chrome({'desired_capabilities': {'key': 'value'}}, url)
+        self.assertEqual(driver, expected_webdriver)
+
     def test_chrome_healdless(self):
         expected_webdriver = mock()
         options = mock()
@@ -171,6 +209,18 @@ class WebDriverCreatorTests(unittest.TestCase):
         driver = self.creator.create_firefox({'desired_capabilities': capabilities}, url, None)
         self.assertEqual(driver, expected_webdriver)
 
+    def test_firefox_remote_caps_no_browsername(self):
+        url = 'http://localhost:4444/wd/hub'
+        profile = mock()
+        when(webdriver).FirefoxProfile().thenReturn(profile)
+        expected_webdriver = mock()
+        capabilities = {'browserName': 'firefox', 'version': '66.02'}
+        when(webdriver).Remote(command_executor=url,
+                               browser_profile=profile, options=None,
+                               desired_capabilities=capabilities).thenReturn(expected_webdriver)
+        driver = self.creator.create_firefox({'capabilities': {"version": "66.02"}}, url, None)
+        self.assertEqual(driver, expected_webdriver)
+
     def test_firefox_profile(self):
         expected_webdriver = mock()
         profile = mock()
@@ -201,8 +251,9 @@ class WebDriverCreatorTests(unittest.TestCase):
         profile = mock()
         when(webdriver).FirefoxProfile().thenReturn(profile)
         remote_url = 'localhost:4444'
+        capabilities = {'browserName': 'firefox', 'key': 'value'}
         when(webdriver).Remote(command_executor=remote_url, options=options,
-                               desired_capabilities={'key': 'value'},
+                               desired_capabilities=capabilities,
                                browser_profile=profile,).thenReturn(expected_webdriver)
         driver = self.creator.create_headless_firefox({'capabilities': {'key': 'value'}}, remote_url, None)
         verify(options).set_headless()
@@ -254,6 +305,16 @@ class WebDriverCreatorTests(unittest.TestCase):
         driver = self.creator.create_ie({'capabilities': capabilities}, url)
         self.assertEqual(driver, expected_webdriver)
 
+    def test_ie_no_browser_name(self):
+        url = 'http://localhost:4444/wd/hub'
+        expected_webdriver = mock()
+        capabilities = {'browserName': 'internet explorer', 'key': 'value'}
+        when(webdriver).Remote(command_executor=url, browser_profile=None,
+                               desired_capabilities=capabilities,
+                               options=None).thenReturn(expected_webdriver)
+        driver = self.creator.create_ie({'capabilities': {'key': 'value'}}, url)
+        self.assertEqual(driver, expected_webdriver)
+
     def test_edge(self):
         expected_webdriver = mock()
         when(webdriver).Edge().thenReturn(expected_webdriver)
@@ -280,6 +341,15 @@ class WebDriverCreatorTests(unittest.TestCase):
         driver = self.creator.create_edge({'capabilities': capabilities}, url)
         self.assertEqual(driver, expected_webdriver)
 
+    def test_edge_no_browser_name(self):
+        url = 'http://localhost:4444/wd/hub'
+        expected_webdriver = mock()
+        capabilities = {'browserName': 'MicrosoftEdge', 'key': 'value'}
+        when(webdriver).Remote(command_executor=url, browser_profile=None,
+                               desired_capabilities=capabilities,
+                               options=None).thenReturn(expected_webdriver)
+        driver = self.creator.create_edge({'capabilities': {'key': 'value'}}, url)
+        self.assertEqual(driver, expected_webdriver)
 
     def test_opera(self):
         expected_webdriver = mock()
@@ -307,6 +377,16 @@ class WebDriverCreatorTests(unittest.TestCase):
         driver = self.creator.create_opera({'desired_capabilities': capabilities}, url)
         self.assertEqual(driver, expected_webdriver)
 
+    def test_opera_no_browser_name(self):
+        url = 'http://localhost:4444/wd/hub'
+        expected_webdriver = mock()
+        capabilities = {'browserName': 'opera', 'key': 'value'}
+        when(webdriver).Remote(command_executor=url, browser_profile=None,
+                               desired_capabilities=capabilities,
+                               options=None).thenReturn(expected_webdriver)
+        driver = self.creator.create_opera({'desired_capabilities': {'key': 'value'}}, url)
+        self.assertEqual(driver, expected_webdriver)
+
     def test_safari(self):
         expected_webdriver = mock()
         when(webdriver).Safari().thenReturn(expected_webdriver)
@@ -333,6 +413,16 @@ class WebDriverCreatorTests(unittest.TestCase):
         driver = self.creator.create_safari({'desired_capabilities': capabilities}, url)
         self.assertEqual(driver, expected_webdriver)
 
+    def test_safari_no_broser_name(self):
+        url = 'http://localhost:4444/wd/hub'
+        expected_webdriver = mock()
+        capabilities = {'browserName': 'safari', 'key': 'value'}
+        when(webdriver).Remote(command_executor=url, browser_profile=None,
+                               desired_capabilities=capabilities,
+                               options=None).thenReturn(expected_webdriver)
+        driver = self.creator.create_safari({'desired_capabilities': {'key': 'value'}}, url)
+        self.assertEqual(driver, expected_webdriver)
+
     def test_phantomjs(self):
         expected_webdriver = mock()
         when(webdriver).PhantomJS().thenReturn(expected_webdriver)
@@ -359,6 +449,16 @@ class WebDriverCreatorTests(unittest.TestCase):
         driver = self.creator.create_phantomjs({'desired_capabilities': capabilities}, url)
         self.assertEqual(driver, expected_webdriver)
 
+    def test_phantomjs_no_browser_name(self):
+        url = 'http://localhost:4444/wd/hub'
+        expected_webdriver = mock()
+        capabilities = {'browserName': 'phantomjs', 'key': 'value'}
+        when(webdriver).Remote(command_executor=url, browser_profile=None,
+                               desired_capabilities=capabilities,
+                               options=None).thenReturn(expected_webdriver)
+        driver = self.creator.create_phantomjs({'desired_capabilities': {'key': 'value'}}, url)
+        self.assertEqual(driver, expected_webdriver)
+
     def test_htmlunit_no_caps(self):
         caps = webdriver.DesiredCapabilities.HTMLUNIT
         expected_webdriver = mock()
@@ -369,7 +469,7 @@ class WebDriverCreatorTests(unittest.TestCase):
         driver = self.creator.create_htmlunit({}, None)
         self.assertEqual(driver, expected_webdriver)
 
-    def test_htmlunit_no_caps(self):
+    def test_htmlunit_remote_caps(self):
         caps = {"browserName": "htmlunit"}
         expected_webdriver = mock()
         when(webdriver).Remote(command_executor='None',
@@ -379,8 +479,18 @@ class WebDriverCreatorTests(unittest.TestCase):
         driver = self.creator.create_htmlunit({'desired_capabilities': caps}, None)
         self.assertEqual(driver, expected_webdriver)
 
+    def test_htmlunit_no_browser_name(self):
+        capabilities = {'browserName': 'htmlunit', 'key': 'value'}
+        expected_webdriver = mock()
+        when(webdriver).Remote(command_executor='None',
+                               desired_capabilities=capabilities,
+                               browser_profile=None,
+                               options=None).thenReturn(expected_webdriver)
+        driver = self.creator.create_htmlunit({'desired_capabilities': {'key': 'value'}}, None)
+        self.assertEqual(driver, expected_webdriver)
+
     def test_htmlunit_with_js(self):
-        caps = webdriver.DesiredCapabilities.HTMLUNITWITHJS
+        caps = webdriver.DesiredCapabilities.HTMLUNITWITHJS.copy()
         expected_webdriver = mock()
         when(webdriver).Remote(command_executor='None',
                                desired_capabilities=caps,
@@ -389,6 +499,16 @@ class WebDriverCreatorTests(unittest.TestCase):
         driver = self.creator.create_htmlunit_with_js({}, None)
         self.assertEqual(driver, expected_webdriver)
 
+    def test_htmlunit_with_js_no_browser_name(self):
+        capabilities = {'browserName': 'htmlunit', 'key': 'value'}
+        expected_webdriver = mock()
+        when(webdriver).Remote(command_executor='None',
+                               desired_capabilities=capabilities,
+                               browser_profile=None,
+                               options=None).thenReturn(expected_webdriver)
+        driver = self.creator.create_htmlunit_with_js({'desired_capabilities': {'key': 'value'}}, None)
+        self.assertEqual(driver, expected_webdriver)
+
     def test_android(self):
         caps = webdriver.DesiredCapabilities.ANDROID
         expected_webdriver = mock()
@@ -399,6 +519,16 @@ class WebDriverCreatorTests(unittest.TestCase):
         driver = self.creator.create_android({}, None)
         self.assertEqual(driver, expected_webdriver)
 
+    def test_android_no_browser_name(self):
+        capabilities = {'browserName': 'android', 'key': 'value'}
+        expected_webdriver = mock()
+        when(webdriver).Remote(command_executor='None',
+                               desired_capabilities=capabilities,
+                               browser_profile=None,
+                               options=None).thenReturn(expected_webdriver)
+        driver = self.creator.create_android({'desired_capabilities': {'key': 'value'}}, None)
+        self.assertEqual(driver, expected_webdriver)
+
     def test_iphone(self):
         caps = webdriver.DesiredCapabilities.IPHONE
         expected_webdriver = mock()
@@ -409,6 +539,16 @@ class WebDriverCreatorTests(unittest.TestCase):
         driver = self.creator.create_iphone({}, None)
         self.assertEqual(driver, expected_webdriver)
 
+    def test_iphone_no_browser_name(self):
+        capabilities = {'browserName': 'iPhone', 'key': 'value'}
+        expected_webdriver = mock()
+        when(webdriver).Remote(command_executor='None',
+                               desired_capabilities=capabilities,
+                               browser_profile=None,
+                               options=None).thenReturn(expected_webdriver)
+        driver = self.creator.create_iphone({'desired_capabilities': {'key': 'value'}}, None)
+        self.assertEqual(driver, expected_webdriver)
+
     def test_create_driver_chrome(self):
         expected_webdriver = mock()
         when(webdriver).Chrome(options=None).thenReturn(expected_webdriver)