Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

phpBrowser click with url parameters #1891

Closed
alexwhitman opened this issue May 1, 2015 · 4 comments
Closed

phpBrowser click with url parameters #1891

alexwhitman opened this issue May 1, 2015 · 4 comments
Labels

Comments

@alexwhitman
Copy link

@alexwhitman alexwhitman commented May 1, 2015

With the following test page:

<!doctype html>
<html>
    <head>
        <title>Select Test</title>
    </head>
    <body>
        <form method="get" action="index.html">
            <input type="radio" id="radio1" name="simple-radio-input" value="1" />
            <input type="radio" id="radio2" name="simple-radio-input" value="2" />
            <input type="submit" value="submit" />
        </form>
    </body>
</html>

And the following test:

    public function testSimpleRadio(WebGuy $I)
    {
        $I->wantTo('get this thing working');
        $I->amOnPage('/temp/index.html?a=b');
        $I->seeElement('input[name="simple-radio-input"]');
        $I->fillField('input[name="simple-radio-input"]', 2);
        $I->seeInField('input[name="simple-radio-input"]', 2);
        $I->click('submit');
        $url = $I->grabFromCurrentUrl();
        var_dump($url);exit;
        $I->seeInCurrentUrl('simple-radio-input=2');
    }

The following string is dumped for $url:
string(58) "/temp/index.html?a=b?simple-radio-input=2&=submit"

Notice how the url contains both the original page parameters and the form submission parameters. This then breaks anything checking the value of simple-radio-input as strictly this isn't set and a is set to b?simple-radio-input=2.

@DavertMik DavertMik added the PhpBrowser label May 1, 2015
@alexwhitman
Copy link
Author

@alexwhitman alexwhitman commented May 1, 2015

I figured out why this breaks and I have a fix but I wasn't sure which branch to create a pull request branch from. I created one from master but a lot of the tests fail with or without the change.

The change is simple so I'll let someone else merge it:

In Lib/InnerBrowser.php, in the function getAbsoluteUrlFor the current url for the page is taken including query parameters and merged with the form action uri. The query parameters need to be unset before merging so the function ends up being:

    protected function getAbsoluteUrlFor($uri)
    {
        $currentUrl = $this->client->getHistory()->current()->getUri();
        if (empty($uri) || $uri === '#') {
            return $currentUrl;
        }
        $build = parse_url($currentUrl);
        if (isset($build['query'])) {
            unset($build['query']);         // This line fixes the issue
        }
        $uriParts = parse_url($uri);
        if ($build === false) {
            throw new TestRuntime("URL '$currentUrl' is malformed");
        } elseif ($uriParts === false) {
            throw new TestRuntime("URI '$uri' is malformed");
        }

        $abs = $this->mergeUrls($build, $uriParts);
        return \GuzzleHttp\Url::buildUrl($abs);
    }
@zbateson
Copy link
Member

@zbateson zbateson commented May 1, 2015

Hi alexwhitman,

I must admit I'm puzzled why you're seeing this result and why unsetting $build['query'] would help. In mergeUrls, $build['query'] would be overwritten by the value of $uriParts['query']. In your example, the value of $uriParts['query'] should be set to ['simple-radio-input' => '2'] upon submission.

Could you run with --debug and show me the output?

For future reference, the 'current' branch is 2.0. The master branch is the next release version - there's a "Contributing" section in /readme.md. I think, in this instance, the issue is probably elsewhere :)

@zbateson
Copy link
Member

@zbateson zbateson commented May 1, 2015

Also, what is the configuration value for PhpBrowser's url config? Is it just 'http://localhost'?

@zbateson
Copy link
Member

@zbateson zbateson commented May 1, 2015

Never mind, I'm reproducing it successfully now.

zbateson added a commit to zbateson/Codeception that referenced this issue May 1, 2015
URLs were not properly constructed for GET form submissions on
URLs requested with query parameters.  Fixes Codeception#1891
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
3 participants
You can’t perform that action at this time.