Add command to retrieve URL modified by Javascript #212

Closed
wants to merge 1 commit into
from

Projects

None yet

4 participants

@joefiorini

If I use pushState to change the URL Browser#current_url still returns
the URL as it was after the original request. This commit adds a command
for RequestedUrl, which is the QtWebkit method for retrieving a URL
modified by Javascript or a redirect.

@joefiorini joefiorini Add command to retrieve URL modified by Javascript
If I use pushState to change the URL Browser#current_url still returns
the URL as it was after the original request. This commit adds a command
for RequestedUrl, which is the QtWebkit method for retrieving a URL
modified by Javascript or a redirect.
89f2cf0
@tristandunn

Seems like the current_url and requested_url behavior would make more sense the other way around.

@joefiorini

Completely agree, however it's directly mirroring the QtWebkit API (http://doc.qt.nokia.com/latest/qwebframe.html#requestedUrl-prop). I prefer that transparency over changing the language set in place by the Qt authors.

@joefiorini

Although thinking more about it, maybe current_url could become requested_url and we could implement url to return QWebFrame#url (which current_url currently uses). Thoughts?

@halogenandtoast halogenandtoast commented on the diff Nov 16, 2011
src/webkit_server.pro
@@ -33,6 +34,11 @@ HEADERS = \
SetProxy.h \
SOURCES = \
+<<<<<<< HEAD
+=======
+ RequestedUrl.cpp \
+ ConsoleMessages.cpp \
+>>>>>>> 3d0768c... Add command to retrieve URL modified by Javascript
@halogenandtoast
halogenandtoast Nov 16, 2011

You left in this merge conflict.

@joefiorini
joefiorini Nov 16, 2011

Agh! Thanks for catching that. I'll fix and push today.

@halogenandtoast

Merged your branch without the merge conflict into master.

@marcisme

Perhaps I'm missing something, but doesn't this leave capybara-webkit inconsistent with other Capybara drivers? Personally I have some request specs that assert "current_path.should == blah" that fail with this driver because of the presence of url fragments that do not exist with say the Selenium driver.

I'm not entirely convinced this shouldn't be viewed as some kind of QtWebKit bug, as if you run history.replaceState in other WebKit-based browsers (Safari & Chrome), the URL in the address bar changes, while in QtWeb, which appears to be built with QtWebKit, it does not.

@halogenandtoast

@marcisme right now current_path will return the paths sans fragments. current_url will return the original url with fragments and requested_url will return the current url as set by javascript.

For example if I visit "/?foo=bar#baz and have a javascript call window.history.pushState('', '', '/qux') on the page: current_path == "/" and current_url == "http://myhost/?foo=bar#baz" and requested_url == "/qux". However when Selenium runs the same test it will spit out current_path == "/qux" and current_url = "http://myhost/qux" so you may be right.

For now I'm going to leave this pull request in, but I'm open to more discussion about these methods and how best to handle them.

@tristandunn

I still don't think it makes sense for current_url to return the requested URL and requested_url to return the original URL. The case for following QtWebkit API doesn't make sense to me either, since you don't need to know anything about QtWebkit to use capybara-webkit, it's a Capybara driver.

Edit: I may have confused myself here. Refer to my original comment if I mistyped.

@halogenandtoast

@tristandunn I think this discussion needs to boil down to the actual values these methods should return. For example:

If visiting "/?foo=bar#baz" that contains window.history.pushState('', '', '/qux') what should be the values for current_url,current_path, and requested_url.

If I do what selenium is currently doing then current_url and current_path will both end with "/qux"

@marcisme

@halogenandtoast I think the value of current_url is most important.

The current_path method is based on current_url and is implemented in Capybara itself, so it should "just work" if current_url is right.

The requested_url method does not appear to be part of the Capybara API, so I see it as more for internal use to access the corresponding property in QWebFrame.

Simply implementing current_url based on requested_url would do the right thing in the case of pushState/replaceState, but unfortunately it returns the wrong value (original URL) in the case of a redirect.

Here are the values I think current_url should return.

  • QWebFrame->requestedUrl when modified by javascript in the visited page
  • QWebFrame->requestedUrl when modified by javascript via session.execute_script
  • QWebFrame->requestedUrl when modified by javascript via session.evaluate_script
  • QWebFrame->url when not modified by javascript (requestedUrl would be wrong after a redirect)

Let me know if you would prefer an example with literal values.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment