diff --git a/application/front/controllers/TagController.php b/application/front/controllers/TagController.php index 598275b0..a1d5ad5b 100644 --- a/application/front/controllers/TagController.php +++ b/application/front/controllers/TagController.php @@ -35,7 +35,7 @@ public function addTag(Request $request, Response $response, array $args): Respo return $response->withRedirect('./'); } - $currentUrl = parse_url($this->container->environment['HTTP_REFERER']); + $currentUrl = parse_url($referer); parse_str($currentUrl['query'] ?? '', $params); if (null === $newTag) { @@ -71,4 +71,50 @@ public function addTag(Request $request, Response $response, array $args): Respo return $response->withRedirect(($currentUrl['path'] ?? './') .'?'. http_build_query($params)); } + + /** + * Remove a tag from the current search through an HTTP redirection. + * + * @param array $args Should contain `tag` key as tag to remove from current search + */ + public function removeTag(Request $request, Response $response, array $args): Response + { + $referer = $this->container->environment['HTTP_REFERER'] ?? null; + + // If the referrer is not provided, we can update the search, so we failback on the bookmark list + if (empty($referer)) { + return $response->withRedirect('./'); + } + + $tagToRemove = $args['tag'] ?? null; + $currentUrl = parse_url($referer); + parse_str($currentUrl['query'] ?? '', $params); + + if (null === $tagToRemove) { + return $response->withRedirect(($currentUrl['path'] ?? './') .'?'. http_build_query($params)); + } + + // Prevent redirection loop + if (isset($params['removetag'])) { + unset($params['removetag']); + } + + if (isset($params['searchtags'])) { + $tags = explode(' ', $params['searchtags']); + // Remove value from array $tags. + $tags = array_diff($tags, [$tagToRemove]); + $params['searchtags'] = implode(' ', $tags); + + if (empty($params['searchtags'])) { + unset($params['searchtags']); + } + + // We also remove page (keeping the same page has no sense, since the results are different) + unset($params['page']); + } + + $queryParams = count($params) > 0 ? '?' . http_build_query($params) : ''; + + return $response->withRedirect(($currentUrl['path'] ?? './') . $queryParams); + } } diff --git a/index.php b/index.php index 04ec0d73..c0e0c66d 100644 --- a/index.php +++ b/index.php @@ -451,35 +451,7 @@ function renderPage($conf, $pluginManager, $bookmarkService, $history, $sessionM // -------- User clicks on a tag in result count: Remove the tag from the list of searched tags (searchtags=...) if (isset($_GET['removetag'])) { - // Get previous URL (http_referer) and remove the tag from the searchtags parameters in query. - if (empty($_SERVER['HTTP_REFERER'])) { - header('Location: ?'); - exit; - } - - // In case browser does not send HTTP_REFERER - parse_str(parse_url($_SERVER['HTTP_REFERER'], PHP_URL_QUERY), $params); - - // Prevent redirection loop - if (isset($params['removetag'])) { - unset($params['removetag']); - } - - if (isset($params['searchtags'])) { - $tags = explode(' ', $params['searchtags']); - // Remove value from array $tags. - $tags = array_diff($tags, array($_GET['removetag'])); - $params['searchtags'] = implode(' ', $tags); - - if (empty($params['searchtags'])) { - unset($params['searchtags']); - } - - // We also remove page (keeping the same page has no sense, since - // the results are different) - unset($params['page']); - } - header('Location: ?'.http_build_query($params)); + header('Location: ./remove-tag/'. $_GET['removetag']); exit; } @@ -1576,6 +1548,7 @@ function install($conf, $sessionManager, $loginManager) $this->get('/open-search', '\Shaarli\Front\Controller\OpenSearchController:index')->setName('opensearch'); $this->get('/add-tag/{newTag}', '\Shaarli\Front\Controller\TagController:addTag')->setName('add-tag'); + $this->get('/remove-tag/{tag}', '\Shaarli\Front\Controller\TagController:removeTag')->setName('remove-tag'); })->add('\Shaarli\Front\ShaarliMiddleware'); $response = $app->run(true); diff --git a/tests/front/controller/TagControllerTest.php b/tests/front/controller/TagControllerTest.php index 5eea537b..2184cb11 100644 --- a/tests/front/controller/TagControllerTest.php +++ b/tests/front/controller/TagControllerTest.php @@ -157,4 +157,86 @@ public function testAddTagWithoutNewTagWithoutReferer(): void static::assertSame(302, $result->getStatusCode()); static::assertSame(['./'], $result->getHeader('location')); } + + public function testRemoveTagWithoutMatchingTag(): void + { + $this->createValidContainerMockSet(); + + $this->container->environment = ['HTTP_REFERER' => 'http://shaarli/controller/?searchtags=def']; + + $request = $this->createMock(Request::class); + $response = new Response(); + + $tags = ['tag' => 'abc']; + + $result = $this->controller->removeTag($request, $response, $tags); + + static::assertInstanceOf(Response::class, $result); + static::assertSame(302, $result->getStatusCode()); + static::assertSame(['/controller/?searchtags=def'], $result->getHeader('location')); + } + + public function testRemoveTagWithoutTagsearch(): void + { + $this->createValidContainerMockSet(); + + $this->container->environment = ['HTTP_REFERER' => 'http://shaarli/controller/']; + + $request = $this->createMock(Request::class); + $response = new Response(); + + $tags = ['tag' => 'abc']; + + $result = $this->controller->removeTag($request, $response, $tags); + + static::assertInstanceOf(Response::class, $result); + static::assertSame(302, $result->getStatusCode()); + static::assertSame(['/controller/'], $result->getHeader('location')); + } + + public function testRemoveTagWithoutReferer(): void + { + $this->createValidContainerMockSet(); + + $request = $this->createMock(Request::class); + $response = new Response(); + + $tags = ['tag' => 'abc']; + + $result = $this->controller->removeTag($request, $response, $tags); + + static::assertInstanceOf(Response::class, $result); + static::assertSame(302, $result->getStatusCode()); + static::assertSame(['./'], $result->getHeader('location')); + } + + public function testRemoveTagWithoutTag(): void + { + $this->createValidContainerMockSet(); + + $this->container->environment = ['HTTP_REFERER' => 'http://shaarli/controller/?searchtag=abc']; + + $request = $this->createMock(Request::class); + $response = new Response(); + + $result = $this->controller->removeTag($request, $response, []); + + static::assertInstanceOf(Response::class, $result); + static::assertSame(302, $result->getStatusCode()); + static::assertSame(['/controller/?searchtag=abc'], $result->getHeader('location')); + } + + public function testRemoveTagWithoutTagWithoutReferer(): void + { + $this->createValidContainerMockSet(); + + $request = $this->createMock(Request::class); + $response = new Response(); + + $result = $this->controller->removeTag($request, $response, []); + + static::assertInstanceOf(Response::class, $result); + static::assertSame(302, $result->getStatusCode()); + static::assertSame(['./'], $result->getHeader('location')); + } } diff --git a/tpl/default/linklist.html b/tpl/default/linklist.html index d8a15823..e574a109 100644 --- a/tpl/default/linklist.html +++ b/tpl/default/linklist.html @@ -94,7 +94,9 @@ {'tagged'|t} {loop="$exploded_tags"} - {$value} + + {$value} + {/loop} {/if} diff --git a/tpl/vintage/linklist.html b/tpl/vintage/linklist.html index 8f1ded95..502abcf9 100644 --- a/tpl/vintage/linklist.html +++ b/tpl/vintage/linklist.html @@ -66,7 +66,7 @@ tagged {loop="$exploded_tags"} - {$value} x + {$value} x {/loop} {elseif="$search_tags === false"}