Support for other browsers #9
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
equalsraf
force-pushed
the
tb-other-browsers
branch
3 times, most recently
from
80212ec to
11aedcf
Compare
equalsraf
changed the title
|
This looks like great progress! On my phone now but I'll take a look at the diffs later. |
|
This is really good work, I'd like to support other browsers, especially Chrome. Opera and webkitgtk sound nice, but could be added later. I think I put a note in README.md or lib.rs about browser support, that will need updating when ChromeDriver goes in. A few suggestions for you here. Please feel free to give alternatives. |
equalsraf
force-pushed
the
tb-other-browsers
branch
from
bb64f9e to
7b7e27e
Compare
Use stderrlog to log to stderr when the user increases verbosity with -v. With -vvvv TRACE messages from the webdriver_client are visible.
The required attribute is not necessary. The "capabilities" and "desiredCapabilities" attribute is mandatory by some servers, the spec describes this as > Let capabilities request be the result of getting the property "capabilities" from parameters. > 1. If capabilities request is not a JSON object, return error with error code invalid argument. but at least WebKitWebDriver refuses to proceed if the the attribute is not available. To be safe I just set it to an actual object instead of null.
By default chromedriver is NOT compliant with the w3c spec. But one can request w3c compliance with a capability extension included in the new session command payload.
This commit adds a driver launcher for the chromedrivr, the type is mostly the same as the GeckoDriver type minus the custom binary option that chromedriver does not have. Added a new option --driver/-D to the `www` cli to specify the desired driver (chromedriver or, the default geckodriver).
One of the pieces of the api that is very well documented is frame/element handling - this commit adds some basic docstrings to the frame methods, and makes the Element::new() method public. For examples I've addedd two more commands to the www binary - `frames` lists iframes in a page with their reference id - `switchframe ID` switches the current context to the selected frame - `switchframe` switches to the toplevel context This still falls short in two areas - there is no way to determine the current frame - AFAIK there is no command for this in the webdriver spec - we are missing a method to switch to the parent frame
I can't find a reference to this attribute in the spec. And I cant remember why I added it.
The switch frame command failed to execute in chrome, the error message is pretty explicit it requires the payload of the method to carry the `ELEMENT` attribute instead of the attribute from the w3c spec (element-6066-11e4-a52e-4f735466cecf). I think the upstream issue is https://bugs.chromium.org/p/chromedriver/issues/detail?id=1992 The proposed workaround for this is to also serialize the ElementReference with an ELEMENT attribute. There is the risk of clashing with other implementations but so far have I have encoutered no issues. Removed the serialization test from the messages module.
Included some tests for frame switching. Took the opportunity do add some docstrings and made the ref constructor public.
equalsraf
force-pushed
the
tb-other-browsers
branch
from
7b7e27e to
7b330d4
Compare
equalsraf
added a commit
to equalsraf/webdriver_client_rust
that referenced
this pull request
Jan 30, 2018
This commit is just meant to verify that fluffysquirrels#9 works. Some tests fail due to diferences between the output of firefox and chrome.
equalsraf
force-pushed
the
tb-other-browsers
branch
from
7b330d4 to
752a6f2
Compare
|
Ok I've moved all of the travis bits to #14. I think i covered all you comments, except the one about avoiding the 'goog:chromeOptions' in the session cmd constructor. BTW github seems to hide away the comments made in a commit when I rebased, original links for the commits and comments |
|
This looks great, very happy for it to be merged. Wow, github hiding those previous comments when you rebase is incredibly annoying, I've not noticed how bad it was before. What I'd expect is a new entry to show up with the old commits greyed out or something. |
|
I created #16 "Consider refactoring browser-specific capabilities on creating a new session" to discuss the 'goog:chromeOptions' in the session cmd constructor. |
fluffysquirrels
pushed a commit
that referenced
this pull request
Jan 31, 2018
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Hi, I've been looking into using this with other browsers
wwwjust because it makes debugging easierI also tried to test with other implementations, but failed for unrelated reasons
Other than that I've added tests using the chrome driver, but it took some fiddling with travis likely because of this issue in travis