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

[DomCrawler] UriResolver support path with columns #52559

Closed
wants to merge 8 commits into from

Conversation

vdauchy
Copy link
Contributor

@vdauchy vdauchy commented Nov 12, 2023

Q A
Branch? 6.3
Bug fix? yes
New feature? no
Deprecations? no
Issues Fix UriResolver bad handling of column in path
License MIT

Resolving links on pages using weird pagination like: https://localhost/domain/search/page:5 fails due to : making

var_dump(parse_url('/page:1', \PHP_URL_SCHEME));

Return false (and not null as expected in the code).

This simply ensure the absolute URL is returned only if the SCHEME is found (ie a string is returned by parse_url).

@derrabus
Copy link
Member

Is 5.4 not affected by this bug?

@carsonbot carsonbot changed the title UriResolver support path with columns [DomCrawler] UriResolver support path with columns Nov 12, 2023
…derer)

This PR was merged into the 5.4 branch.

Discussion
----------

[Validator] Updated Lithuanian translations

| Q             | A
| ------------- | ---
| Branch?       | 5.4
| Bug fix?      | no
| New feature?  | no
| Deprecations? | no
| Issues        | Fix symfony#51948
| License       | MIT

Missing Lithuanian translations with help from a native speaker.

Commits
-------

5938c05 [Validator] updated Lithuanian translation
@vdauchy
Copy link
Contributor Author

vdauchy commented Nov 13, 2023

@derrabus From https://github.com/symfony/symfony/blob/5.4/src/Symfony/Component/DomCrawler/UriResolver.php I think the but does exist in 5.4.

Shall I re-do the a PR targeting 5.4 branch then the patch will be automatically applied upstream ?

@derrabus
Copy link
Member

Yes, please target 5.4 then. No need to "re-do" the PR, just rebase this one and change the target.

@vdauchy
Copy link
Contributor Author

vdauchy commented Nov 14, 2023

Moved to #52579

@vdauchy vdauchy closed this Nov 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants