mozilla / geckodriver

WebDriver for Firefox
https://firefox-source-docs.mozilla.org/testing/geckodriver/
Mozilla Public License 2.0
7.2k stars 1.52k forks source link

locating with empty tag name should error #2007

Closed titusfortner closed 2 years ago

titusfortner commented 2 years ago

System

Testcase

Selenium Python, Ruby & JS have been converting tag name selectors to CSS selectors, which according to w3c is apparently not quite correct.

When locating by an empty tag name, Firefox returns element not found. I think Chrome is doing the correct thing by returning "invalid selector" error.

driver.find_element(tag_name: '')
juliandescottes commented 2 years ago

I am not sure about this one.

From the tagname strategy https://www.w3.org/TR/webdriver/#tag-name, we should defer to

calling getElementsByTagName with start node as this and selector as the argument.

(no validation of selector there)

And I also don't see any validation of the qualifiedName argument in getelementsbytagname.

And neither Chrome or Firefox throw if you use getElementsByTagName("") on any node.

Is there a specific part in the spec which makes you think this should lead to an error?

titusfortner commented 2 years ago

Chrome comes back with invalid element selector for both find_element and find_elements

irb(main):018:0> driver = Selenium
irb(main):018:0> driver = Selenium::WebDriver.for :chrome 
=> #<Selenium::WebDriver::Chrome::Driver:0x..f6033c0f9f52cbb6 browser=:chrome>
irb(main):019:0> Selenium::WebDriver.logger.level = :info
=> :info
irb(main):020:0> driver.find_element(tag_name: '')
2022-09-21 17:23:46 INFO Selenium -> POST session/34a5630342ee49324562e5e19522ad02/element
2022-09-21 17:23:46 INFO Selenium    >>> http://127.0.0.1:9515/session/34a5630342ee49324562e5e19522ad02/element | {"using":"tag name","value":""}
2022-09-21 17:23:46 INFO Selenium <- {"value":{"error":"invalid selector","message":"invalid selector: Unable to locate an element with the tagName \"\"\n  (Session info: chrome=105.0.5195.125)","stacktrace":"0   chromedriver                        0x00000001020b4788 chromedriver + 4515720\n1   chromedriver                        0x00000001020389d3 chromedriver + 4008403\n2   chromedriver                        0x0000000101ccb12a chromedriver + 413994\n3   chromedriver                        0x0000000101cce1b7 chromedriver + 426423\n4   chromedriver                        0x0000000101cce05f chromedriver + 426079\n5   chromedriver                        0x0000000101cce31c chromedriver + 426780\n6   chromedriver                        0x0000000101d02587 chromedriver + 640391\n7   chromedriver                        0x0000000101d02a61 chromedriver + 641633\n8   chromedriver                        0x0000000101d34f64 chromedriver + 847716\n9   chromedriver                        0x0000000101d1f7fd chromedriver + 759805\n10  chromedriver                        0x0000000101d32bd9 chromedriver + 838617\n11  chromedriver                        0x0000000101d1f603 chromedriver + 759299\n12  chromedriver                        0x0000000101cf5990 chromedriver + 588176\n13  chromedriver                        0x0000000101cf6a75 chromedriver + 592501\n14  chromedriver                        0x00000001020846cd chromedriver + 4318925\n15  chromedriver                        0x0000000102088f35 chromedriver + 4337461\n16  chromedriver                        0x00000001020901ff chromedriver + 4366847\n17  chromedriver                        0x0000000102089c5a chromedriver + 4340826\n18  chromedriver                        0x000000010205fc2c chromedriver + 4168748\n19  chromedriver                        0x00000001020a64f8 chromedriver + 4457720\n20  chromedriver                        0x00000001020a6693 chromedriver + 4458131\n21  chromedriver                        0x00000001020bba3e chromedriver + 4545086\n22  libsystem_pthread.dylib             0x00007fff204378fc _pthread_start + 224\n23  libsystem_pthread.dylib             0x00007fff20433443 thread_start + 15\n"}}      
0   chromedriver                        0x00000001020b4788 chromedriver + 4515720: invalid selector: Unable to locate an element with the tagName "" (Selenium::WebDriver::Error::InvalidSelectorError)                                        
  (Session info: chrome=105.0.5195.125)                                      
        from 1   chromedriver                        0x00000001020389d3 chromedriver + 4008403
    from 2   chromedriver                        0x0000000101ccb12a chromedriver + 413994
    from 3   chromedriver                        0x0000000101cce1b7 chromedriver + 426423
    from 4   chromedriver                        0x0000000101cce05f chromedriver + 426079
    from 5   chromedriver                        0x0000000101cce31c chromedriver + 426780
    from 6   chromedriver                        0x0000000101d02587 chromedriver + 640391
    from 7   chromedriver                        0x0000000101d02a61 chromedriver + 641633
    from 8   chromedriver                        0x0000000101d34f64 chromedriver + 847716
    from 9   chromedriver                        0x0000000101d1f7fd chromedriver + 759805
    from 10  chromedriver                        0x0000000101d32bd9 chromedriver + 838617
    from 11  chromedriver                        0x0000000101d1f603 chromedriver + 759299
    from 12  chromedriver                        0x0000000101cf5990 chromedriver + 588176
    from 13  chromedriver                        0x0000000101cf6a75 chromedriver + 592501
    from 14  chromedriver                        0x00000001020846cd chromedriver + 4318925
    from 15  chromedriver                        0x0000000102088f35 chromedriver + 4337461
    from 16  chromedriver                        0x00000001020901ff chromedriver + 4366847
    ... 20 levels...
irb(main):021:0> driver.find_elements(tag_name: '')
2022-09-21 17:23:50 INFO Selenium -> POST session/34a5630342ee49324562e5e19522ad02/elements
2022-09-21 17:23:50 INFO Selenium    >>> http://127.0.0.1:9515/session/34a5630342ee49324562e5e19522ad02/elements | {"using":"tag name","value":""}
2022-09-21 17:23:50 INFO Selenium <- {"value":{"error":"invalid selector","message":"invalid selector: Unable to locate an element with the tagName \"\"\n  (Session info: chrome=105.0.5195.125)","stacktrace":"0   chromedriver                        0x00000001020b4788 chromedriver + 4515720\n1   chromedriver                        0x00000001020389d3 chromedriver + 4008403\n2   chromedriver                        0x0000000101ccb12a chromedriver + 413994\n3   chromedriver                        0x0000000101cce1b7 chromedriver + 426423\n4   chromedriver                        0x0000000101cce05f chromedriver + 426079\n5   chromedriver                        0x0000000101cce31c chromedriver + 426780\n6   chromedriver                        0x0000000101d02587 chromedriver + 640391\n7   chromedriver                        0x0000000101d02a61 chromedriver + 641633\n8   chromedriver                        0x0000000101d34fa1 chromedriver + 847777\n9   chromedriver                        0x0000000101d1f7fd chromedriver + 759805\n10  chromedriver                        0x0000000101d32bd9 chromedriver + 838617\n11  chromedriver                        0x0000000101d1f603 chromedriver + 759299\n12  chromedriver                        0x0000000101cf5990 chromedriver + 588176\n13  chromedriver                        0x0000000101cf6a75 chromedriver + 592501\n14  chromedriver                        0x00000001020846cd chromedriver + 4318925\n15  chromedriver                        0x0000000102088f35 chromedriver + 4337461\n16  chromedriver                        0x00000001020901ff chromedriver + 4366847\n17  chromedriver                        0x0000000102089c5a chromedriver + 4340826\n18  chromedriver                        0x000000010205fc2c chromedriver + 4168748\n19  chromedriver                        0x00000001020a64f8 chromedriver + 4457720\n20  chromedriver                        0x00000001020a6693 chromedriver + 4458131\n21  chromedriver                        0x00000001020bba3e chromedriver + 4545086\n22  libsystem_pthread.dylib             0x00007fff204378fc _pthread_start + 224\n23  libsystem_pthread.dylib             0x00007fff20433443 thread_start + 15\n"}}
0   chromedriver                        0x00000001020b4788 chromedriver + 4515720: invalid selector: Unable to locate an element with the tagName "" (Selenium::WebDriver::Error::InvalidSelectorError)
  (Session info: chrome=105.0.5195.125)
    from 1   chromedriver                        0x00000001020389d3 chromedriver + 4008403
    from 2   chromedriver                        0x0000000101ccb12a chromedriver + 413994
    from 3   chromedriver                        0x0000000101cce1b7 chromedriver + 426423
    from 4   chromedriver                        0x0000000101cce05f chromedriver + 426079
    from 5   chromedriver                        0x0000000101cce31c chromedriver + 426780
    from 6   chromedriver                        0x0000000101d02587 chromedriver + 640391
    from 7   chromedriver                        0x0000000101d02a61 chromedriver + 641633
    from 8   chromedriver                        0x0000000101d34fa1 chromedriver + 847777
    from 9   chromedriver                        0x0000000101d1f7fd chromedriver + 759805
    from 10  chromedriver                        0x0000000101d32bd9 chromedriver + 838617
    from 11  chromedriver                        0x0000000101d1f603 chromedriver + 759299
    from 12  chromedriver                        0x0000000101cf5990 chromedriver + 588176
    from 13  chromedriver                        0x0000000101cf6a75 chromedriver + 592501
    from 14  chromedriver                        0x00000001020846cd chromedriver + 4318925
    from 15  chromedriver                        0x0000000102088f35 chromedriver + 4337461
    from 16  chromedriver                        0x00000001020901ff chromedriver + 4366847
    ... 20 levels...
irb(main):022:0> 
titusfortner commented 2 years ago

Maybe Firefox is right and Chrome is wrong, but if I'm choosing which I prefer... I'd rather an error that I'm not going to include in an automatic retry loop, because that element can't exist.

juliandescottes commented 2 years ago

From what I can tell, the discrepancy comes from the fact that chromedriver relies on the selenium atoms to implement various find element strategies, whereas geckodriver/marionette usually implement the logic on their own.

And atoms do throw explicitly (with the error you see here) when trying to find an element with tagname set to empty string: https://github.com/SeleniumHQ/selenium/blob/64447d4b03f6986337d1ca8d8b6476653570bcc1/javascript/atoms/locators/tag_name.js#L32 added at https://github.com/SeleniumHQ/selenium/commit/c4312488632ad52cb4afa200adacdf89e8b97630

How do we move forward here? Should we ask to modify the WebDriver spec to align with the Selenium atom implementation? cc @whimboo @jgraham

whimboo commented 2 years ago

Having an empty value for the call to getElementsByTagName is valid as as @juliandescottes pointed out. I don't think that the WebDriver classic spec should be updated here.

Maybe the upcoming update of the atoms (https://github.com/w3c/webdriver/pull/1673) might fix that when Google still want to continue using the atom and updates to that version? But per WebDriver spec the only atoms to use should actually be bot.dom.getVisibleText and bot.dom.isShown. Or do I read that wrong?

CC'ing @sadym-chromium as well.

titusfortner commented 2 years ago

Is it possible to have an element with an empty string as a tag name?

whimboo commented 2 years ago

I think that this is more a question for the webdriver classic specification instead. As of now geckodriver is spec compliant. So maybe we move the discussion to the webdriver repository?

juliandescottes commented 2 years ago

I tried to summarize the discussion in a webdriver issue, feel free to chime in / correct if there's a need. Do we keep this open until the webdriver issue has been discussed or do we close?

whimboo commented 2 years ago

Thanks Julian for filing the issue. I would say we close this issue given that we are following the current spec.