Skip to content

Conversation

@dten
Copy link
Contributor

@dten dten commented Apr 4, 2023

This reverts commit 8bc94c2 .

when we merged your changes into our fork this broke calls of focusWindow

the docs say it should be "name" as it originally was

https://www.selenium.dev/documentation/legacy/json_wire_protocol/#sessionsessionidwindow

note in w3c it is changed to handle, but this package doesn't get anywhere near w3c support

@TeofilC
Copy link
Contributor

TeofilC commented Nov 17, 2023

Could you talk a look at this @thomasjm when you get a chance?

@thomasjm
Copy link
Member

I'm a little confused about this one. I just tried running the tests on one of my projects with this change, and I get this exception:

FailedCommand UnknownError 
        Session: SessionId "a37b03176f365dbf223b614cddad46e9" at "127.0.0.1":8379
        org.openqa.selenium.InvalidArgumentException: invalid argument: 'handle' must be a string
          (Session info: chrome=114.0.5735.198)
        Build info: version: '3.141.59', revision: 'e82be7d358', time: '2018-11-14T08:25:53'
        System info: host: 'desktop2-vm', ip: '127.0.1.1', os.name: 'Linux', os.arch: 'amd64', os.version: '5.15.133.1-microsoft-standard-WSL2', java.version: '11.0.19'
        Driver info: driver.version: unknown
          Test.Sandwich.WebDriver.Types.getJSONResult (src/Test/Sandwich/WebDriver/Types.hs:55)
          Test.WebDriver.Commands.Internal.doCommand (src/Test/WebDriver/Commands/Internal.hs:91)
          Test.WebDriver.Commands.doSessCommand (src/Test/WebDriver/Commands.hs:308)

The logs indicate this is Selenium 3.141.59 and Chrome 114.x.

I gather Selenium does a "handshake" during initial setup and infers whether it should use the JSON Wire Protocol vs. the W3C WebDriver Protocol. But why does Selenium seemingly expect W3C parameters in this case in my setup when many other commands work fine?

@thomasjm
Copy link
Member

Looking at my Selenium logs, I see this line after it receives the capabilities:

20:31:50.813 INFO [ProtocolHandshake.createSession] - Detected dialect: W3C

Do you have a different output? I wonder if your capabilities are putting it into the other dialect.

@dten
Copy link
Contributor Author

dten commented Nov 18, 2023

Chrome defaulted to w3c mode since Chrome 75 at which point this library broke significantly. We've been running with w3c mode configured off since then because, correct me if I'm wrong, but in general this package does not adhere to the w3c spec - it uses the legacy wire protocol.

@thomasjm
Copy link
Member

Can you tell me how you turned w3c mode off? That would be interesting to test.

Yes AFAIK this package is designed to use the legacy wire protocol, though I'm not the original author.

Also, can you tell me any specific commands that fail if you use w3c mode?

@dten
Copy link
Contributor Author

dten commented Nov 18, 2023

you can disable w3c mode with these chrome capabilities

import Data.Aeson (Value (Bool))
import qualified Data.Aeson.KeyMap as KM
import qualified Test.WebDriver as W

W.defaultCaps { W.browser = W.chrome {  W.chromeExperimentalOptions = KM.fromList [ ("w3c", Bool False) ] } }

I'll have to get back to you on what fails without this set

@dten
Copy link
Contributor Author

dten commented Dec 6, 2023

On a related note. Do you have any plans regarding w3c? Would you be up for this package implementing the w3c spec instead of a mix of both but mostly the legacy spec?
We have a lot of tests that use this package and I'm more interested in moving this to w3c rather than change all our tests to another package. I've started looking at what changes are needed for our tests to work with w3c compliant webdriver impls and would be happy to upstream that when done.

#144 has a lot of relevant changes such as removing the unsupported selectors and making commands always send an object, but there's a lot more needed in order to do the error handling properly etc.

@thomasjm
Copy link
Member

Do you have any plans regarding w3c? Would you be up for this package implementing the w3c spec instead of a mix of both but mostly the legacy spec?

That would be great! Maybe we could have a quick design discussion about how to do it? One thing I'm not sure about is whether it would be worthwhile to maintain support for the legacy protocol. It seems like a major change to drop it, but maybe it's a thing of the past by this point?

If we did want to keep the legacy protocol, I wonder if we'd want to make a giant typeclass for all the webdriver commands and then provide an implementation for both flavors.

Another thing I was pondering is trying to autogenerate using an OpenAPI spec: https://github.com/aerokube/selenium-openapi. Writing all the commands out by hand has proven to be error-prone in the past.

@dten
Copy link
Contributor Author

dten commented Dec 13, 2023

On supporting the legacy protocol, I don't see the need. Selenium has supported w3c commands since 3 and even 3 has now dropped out of support. I was surprised to see that testing IE is possible with the w3c commands. Also Mozilla's marionette has never supported legacy.

I do think not ripping up this library and starting again has a benefit though. Might as well just use webdriver-w3c on hackage otherwise.

Another thing is this library already provides support for things that selenium supports that haven't (yet?) made it into the spec. Some things that can be simulated with actions, such as double click, and some cannot such as file upload.

Generating the types always sounds tempting but I've not seen it work out great on the past. Again would probably amount to starting again with this library also for users. Most of the API surface is the same as selenium anyway, the big difference is the actions api, and I suppose the actual error communication. Neither of which are that bad

@dten
Copy link
Contributor Author

dten commented Dec 13, 2023

Also on the non w3c commands currently supported. In w3c mode chrimedriver will error of you try use the legacy doubleClick method because it wants you to use actions, but it will accept file upload no problem because there is no w3c alternative

@dten
Copy link
Contributor Author

dten commented Mar 7, 2025

with the renewed focus on modernisation this MR isn't needed so i'll close

@dten dten closed this Mar 7, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants