Problem/Motivation

HttpKernelUiHelperTrait::getUrl() returns something that looks like:

> http://localhost/node/2

but if you pass that to drupalGet(), it prepends a / to it and you get a broken URL.

Steps to reproduce

Proposed resolution

Not sure. We already added a special case for query links to drupalGet() (#3589626: clickLink in kernel tests erroneously prefixes some paths with /), and adding ANOTHER special case is starting to get complicated.

I have a hunch that maybef the special handling I'd added to the browser at the symfony level was needed after all.

We could make getUrl() return a url without a domain, e.g.:

/node/2

but would that break other things?

Remaining tasks

User interface changes

Introduced terminology

API changes

Data model changes

Release notes snippet

Issue fork drupal-3594522

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

Comments

joachim created an issue. See original summary.

joachim’s picture

> I have a hunch that maybef the special handling I'd added to the browser at the symfony level was needed after all.

The bit I mean is commit 40f2cc07edabd8cf86f67deb8cb25274a4ddb7af in branch 3390193-add-drupalget-with-mink-to-kerneltestbase in the fork for #3390193: Add a drupalGet() method to KernelTestBase.

This added a custom browser class whose job was to convert all URLs to relative:

+<?php
+
+declare(strict_types=1);
+
+namespace Drupal\TestTools\HttpKernel;
+
+use Symfony\Component\HttpKernel\HttpKernelBrowser;
+
+/**
+ * Browserkit client for use in kernel tests.
+ *
+ * This overrides getAbsoluteUri() to skip the conversion from a relative URI to
+ * an absolute URI. We are using this with the HTTP kernel and that needs a
+ * relative URI.
+ */
+class KernelTestHttpKernelBrowser extends HttpKernelBrowser {
+
+  /**
+   * {@inheritdoc}
+   */
+  protected function getAbsoluteUri(string $uri): string {
+    // Preserve the given relative URI.
+    return $uri;
+  }
+
+}

I don't remember the details, but what I **think** this meant was that we let Mink handle converting any URL we passed into its browser, and then at the end we make it into something that the HTTP kernel can work with.

I'm wondering whether with this, we fix this bug and also we no longer need the special handling for anchor links.

joachim’s picture

Yes, restoring that makes this work:

$this->drupalGet($this->getUrl());
joachim’s picture

Issue summary: View changes

Hmm but it doesn't work with relative query links.

And HttpKernelBrowser::getAbsoluteUri() is doing weird stuff to the URLs in that scenario (#3589626: clickLink in kernel tests erroneously prefixes some paths with / didn't add a test for it)

joachim’s picture

Status: Active » Needs work

joachim’s picture

Making a draft MR with some of my work so far.

Overriding getAbsoluteUri() fixes one thing, but breaks another!!!

I've also tried stripping the host from the URL using the PHP 8.5 Uri\Rfc3986\Uri class like this:

  protected function getAbsoluteUri(string $uri): string {
    dump($uri);

    $uri = parent::getAbsoluteUri($uri);
    dump($uri);

    $uri_object = new Uri($uri);

    $uri_object = $uri_object->withScheme(NULL)->withHost(NULL);

    $uri = $uri_object->toRawString();
    dump($uri);

    // dump($uri);
    return $uri;

but in some calls, the call to parent::getAbsoluteUri() is doing WEIRD STUFF to a relative URL.

joachim’s picture

Actually, if we're doing to use Uri\Rfc3986\Uri, then maybe we should use it to manipulate getUrl() to return a path (eg. '/my/page'), that way the manipulation is targeted only where it needs to be done, instead of on every drupalGet() call.

joachim’s picture

Status: Needs work » Needs review

That works!

mstrelan’s picture

I haven't looked at the MR here but my gut feel is that passing Url objects is generally better than passing strings.

joachim’s picture

True, but then we'd be diverging totally from the getUrl() in browser tests.

mstrelan’s picture

What about introducing getUrlObject and getUrlString(bool $absolute) for both kernel and functional, and deprecating getUrl? Then conversions that call getUrl can switch to getUrlObject. This would be similar to the approach of deprecating xpath() in favour of more explicit functions.

joachim’s picture

Ok with getUrlObject().
But getUrlString() on kernel tests has no purpose as an absolute URL

joachim’s picture

Also, what kind of Url object? Drupal URL or PSR URL?

mstrelan’s picture

Whichever one you can pass to drupalGet(), I think durpal