The 2026 Drupal Business Survey is open. Your response will help shape the future of the Drupal business ecosystem.Problem/Motivation
Follow up to #3582769: Add clickLink() to HttpKernelUiHelperTrait.
This doesn't work when the link is just a query string, it assumes you want to go to the front page.
For example:
<a href="?page=1" title="Go to next page" rel="next">
Steps to reproduce
Proposed resolution
@joachim suggested an additional param to drupalGet but I don't think that's necessary. In this case we can just detect the ? as the first character. As mentioned in the other issue, relative paths that are not root-relative are not common in Drupal.
Another suggestion from @joachim was to use getUrl() (added in #3583594: Add getUrl() and (renamed) xpath() helpers to HttpKernelUiHelperTrait) to construct the full path that we pass to drupalGet(), which might be a better approach, but I'm not sure if there are some edge cases if that URL already has a query string, for example.
Remaining tasks
User interface changes
Introduced terminology
API changes
Data model changes
Release notes snippet
| Comment | File | Size | Author |
|---|
Issue fork drupal-3589626
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:
- 3589626-clicklink-in-kernel
changes, plain diff MR !15734
Comments
Comment #2
mstrelan commentedComment #4
mstrelan commentedAdded an MR that detects the question mark prefix and cherry-picked a conversion from #3588363: Convert some tests in Drupal\FunctionalTests namespace to kernel tests as proof that this is working. The rest of that conversion still has a todo, so I don't think we should commit that part here, just leaving it as evidence.
Comment #5
joachim commentedLooks good, and I agree that detecting the ? is the simplest way, but that condition is getting longer and more complicated, so I think it could do with a comment to explain.
Comment #6
needs-review-queue-bot commentedThe Needs Review Queue Bot tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".
This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.
Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.
Comment #7
mstrelan commentedRebased and updated the comment and @param docblock.
Please remember the actual test conversion should not be committed here as it has an outstanding todo, but we need it to demonstrate the clickLink changes.
Comment #8
joachim commentedChange looks good to me but something's failing in the CI.
Comment #9
mstrelan commentedRandom fails. Re-ran twice. I guess this needs a separate MR without the test conversion?
Comment #11
dwwPipeline is now happy. I just rebased to latest main, and left out the two commits with the test conversion. So the MR should be ready to merge now.
p.s. whoops, x-post. Nah, I think having https://git.drupalcode.org/project/drupal/-/pipelines/831418 in here is enough to see the changes working, and we can use the single MR.
Comment #12
dwwEven though I pushed a rebase (and removed 2 commits @mstrelan said should be omitted), so technically I contributed "code", I didn't actually write anything here. 😅 Going to be bold and RTBC it, anyway. The changes are tiny. The new comment is clear. The latest pipeline is green. Previous pipelines (including the now-removed test conversion) show the change is working as intended. This is a good fix that needs to go everywhere the Kernel version of
clickLink()is backported. Let's get it in.Thanks!
-Derek
Comment #16
catchCommitted/pushed to main and cherry-picked to 11.x and 11.4.x, thanks!
Comment #20
mstrelan commentedThis was backported to 10.6 in #3390193: Add a drupalGet() method to KernelTestBase, updating version.