[DomCrawler] Remove the query string and the anchor of the uri of a link #11194
Conversation
benja-M-1
commented
Jun 21, 2014
| Q | A |
|---|---|
| Bug fix? | yes |
| New feature? | no |
| BC breaks? | no |
| Deprecations? | no |
| Tests pass? | yes |
| Fixed tickets | ~ |
| License | MIT |
| Doc PR | ~ |
| @@ -129,7 +121,7 @@ public function getUri() | |||
|
|
|||
| // absolute path | |||
| if ('/' === $uri[0]) { | |||
| return $baseUri.$uri; | |||
| return $this->cleanUri($baseUri).$uri; | |||
There was a problem hiding this comment.
shouldn't this be done before the preg_replace call a few lines above actually ?
There was a problem hiding this comment.
Based on your comment I refactored a bit the code.
|
@fabpot do you want me to squash the first commit and the second in which I renamed the method? |
|
👍 |
|
|
||
| // anchor or query string | ||
| if ('#' === $uri[0] || '?' === $uri[0]) { |
There was a problem hiding this comment.
This change is wrong. In case the link is an anchored link, the query string should not be removed from the current URI, only the fragment
There was a problem hiding this comment.
btw, this case is missing from the testsuite
There was a problem hiding this comment.
hum you are right. I didn't thought about this case. I will change this.
There was a problem hiding this comment.
please also add a test covering the combination of query string and fragment for both types of links
| * | ||
| * @param $uri | ||
| * | ||
| * @return array |
|
once the phpdoc is fixed, 👍 |
|
Thank you @benja-M-1. |
… uri of a link (benja-M-1) This PR was squashed before being merged into the 2.3 branch (closes #11194). Discussion ---------- [DomCrawler] Remove the query string and the anchor of the uri of a link | Q | A | ------------- | --- | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | ~ | License | MIT | Doc PR | ~ Commits ------- fe5d2d1 [DomCrawler] Remove the query string and the anchor of the uri of a link
This PR was merged into the 2.3 branch. Discussion ---------- [DomCrawler] Fix Link docblocks and formatting | Q | A | ------------- | --- | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | - | License | MIT | Doc PR | - re #11194 Commits ------- 5cbe13e [DomCrawler] Fix docblocks and formatting.