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

Simpletest file test adding needless slashes to base path. #4687

Open
bradbulger opened this issue Oct 6, 2020 · 7 comments · May be fixed by backdrop/backdrop#3361
Open

Simpletest file test adding needless slashes to base path. #4687

bradbulger opened this issue Oct 6, 2020 · 7 comments · May be fixed by backdrop/backdrop#3361

Comments

@bradbulger
Copy link

The base path always begins and ends with a slash ('/'). The Simpletest file test code adds slashes to base_path() that aren't needed, so it ends up creating and looking for paths like "//path/to/my/backdrop//core/misc/favicon.ico".

This is marginally a bug since the test is meeting its own expectations, but still.

@jlfranklin
Copy link
Member

jlfranklin commented Oct 6, 2020

There is a bug here, but it's not the double-slash at the beginning. The test claims to be testing protocol-relative URLs, such as //example.org/index.html. This kind of URL says "use the same protocol, http or https, as the current page when connecting to example.org."

But, base_path() returns everything after the hostname. The test should figure out the test site's hostname and include it in the URL.

@bradbulger
Copy link
Author

bradbulger commented Oct 6, 2020

so it should be the base URL minus the protocol? //my.server/path/to/backdrop/admin vs //path/to/backdrop/admin ?

@bradbulger
Copy link
Author

In old issues from Drupal like #777830 they refer to paths like "//files/image.jpg" as protocol-relative, but the external documents they link to match the Wikipedia definition that you linked, "//my.server/files/image.jpg". I guess that's where we got it?

@bradbulger
Copy link
Author

I updated the PR to fix the constructed protocol-relative URLs. Should the tests be doing a backdropGet() to confirm those URLs actually work?

@bradbulger
Copy link
Author

Shouldn't saying "fixes [link to issue]" in the first comment of the PR make it link to this issue?

@gitressa
Copy link

gitressa commented May 2, 2022

I am also puzzled, the syntax looks correct in your PR backdrop/backdrop#336 yet it says "Development - No branches or pull requests" here ...

@gitressa
Copy link

gitressa commented May 2, 2022

Maybe try closing and reopening the PR? (I got the idea from @stpaultim in backdrop/backdrop#3333 (comment)).

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

Successfully merging a pull request may close this issue.

3 participants