Keep list of icons in a separate file for use in the accessibility app

Signed-off-by: Julius Härtl <jus@bitgrid.net>

apps/accessibility/lib/Controller/AccessibilityController.php

@@ -166,8 +166,8 @@ class AccessibilityController extends Controller {
 		$appWebRoot = substr($this->appRoot, strlen($this->serverRoot) - strlen(\OC::$WEBROOT));
 		$css        = $this->rebaseUrls($css, $appWebRoot . '/css');
 
-		if (in_array('themedark', $userValues) && $this->iconsCacher->getCachedCSS() && $this->iconsCacher->getCachedCSS()->getSize() > 0) {
-			$iconsCss = $this->invertSvgIconsColor($this->iconsCacher->getCachedCSS()->getContent());
+		if (in_array('themedark', $userValues) && $this->iconsCacher->getCachedList() && $this->iconsCacher->getCachedList()->getSize() > 0) {
+			$iconsCss = $this->invertSvgIconsColor($this->iconsCacher->getCachedList()->getContent());
 			$css = $css . $iconsCss;
 		}
 

core/Controller/SvgController.php

@@ -31,6 +31,7 @@ use OCP\AppFramework\Http\NotFoundResponse;
 use OCP\AppFramework\Utility\ITimeFactory;
 use OCP\App\IAppManager;
 use OCP\IRequest;
+use OC\Template\IconsCacher;
 
 class SvgController extends Controller {
 
@@ -43,15 +44,20 @@ class SvgController extends Controller {
 	/** @var IAppManager */
 	protected $appManager;
 
+	/** @var IconsCacher */
+	private $iconsCacher;
+
 	public function __construct(string $appName,
 								IRequest $request,
 								ITimeFactory $timeFactory,
-								IAppManager $appManager) {
+								IAppManager $appManager,
+								IconsCacher $iconsCacher) {
 		parent::__construct($appName, $request);
 
 		$this->serverRoot  = \OC::$SERVERROOT;
 		$this->timeFactory = $timeFactory;
 		$this->appManager = $appManager;
+		$this->iconsCacher = $iconsCacher;
 	}
 
 	/**
@@ -114,17 +120,11 @@ class SvgController extends Controller {
 
 		$svg = file_get_contents($path);
 
-		if (is_null($svg)) {
+		if ($svg === null) {
 			return new NotFoundResponse();
 		}
 
-		// add fill (fill is not present on black elements)
-		$fillRe = '/<((circle|rect|path)((?!fill)[a-z0-9 =".\-#():;])+)\/>/mi';
-		$svg = preg_replace($fillRe, '<$1 fill="#' . $color . '"/>', $svg);
-
-		// replace any fill or stroke colors
-		$svg = preg_replace('/stroke="#([a-z0-9]{3,6})"/mi', 'stroke="#' . $color . '"', $svg);
-		$svg = preg_replace('/fill="#([a-z0-9]{3,6})"/mi', 'fill="#' . $color . '"', $svg);
+		$svg = $this->iconsCacher->colorizeSvg($svg, $color);
 
 		$response = new DataDisplayResponse($svg, Http::STATUS_OK, ['Content-Type' => 'image/svg+xml']);
 

lib/private/Template/IconsCacher.php

@@ -52,10 +52,13 @@ class IconsCacher {
 	/** @var string */
 	private $fileName = 'icons-vars.css';
 
+	private $iconList = 'icons-list.template';
+
 	/**
 	 * @param ILogger $logger
 	 * @param Factory $appDataFactory
 	 * @param IURLGenerator $urlGenerator
+	 * @throws \OCP\Files\NotPermittedException
 	 */
 	public function __construct(ILogger $logger,
 								Factory $appDataFactory,
@@ -71,7 +74,7 @@ class IconsCacher {
 		}
 	}
 
-	private function getIconsFromCss(string $css): array{
+	private function getIconsFromCss(string $css): array {
 		preg_match_all($this->iconVarRE, $css, $matches, PREG_SET_ORDER);
 		$icons = [];
 		foreach ($matches as $icon) {
@@ -80,80 +83,101 @@ class IconsCacher {
 
 		return $icons;
 	}
+
 	/**
-	 * Parse and cache css
-	 *
 	 * @param string $css
+	 * @return string
+	 * @throws NotFoundException
+	 * @throws \OCP\Files\NotPermittedException
 	 */
-	public function setIconsCss(string $css) {
+	public function setIconsCss(string $css): string {
 
-		$cachedFile = $this->getCachedCSS();
+		$cachedFile = $this->getCachedList();
 		if (!$cachedFile) {
 			$currentData = '';
+			$cachedFile = $this->folder->newFile($this->iconList);
 		} else {
 			$currentData = $cachedFile->getContent();
 		}
 
-		// remove :root
-		$currentData = str_replace([':root {', '}'], '', $currentData);
+		$cachedVarsCssFile = $this->getCachedCSS();
+		if (!$cachedVarsCssFile) {
+			$cachedVarsCssFile = $this->folder->newFile($this->fileName);
+		}
 
 		$icons = $this->getIconsFromCss($currentData . $css);
 
 		$data = '';
+		$list = '';
 		foreach ($icons as $icon => $url) {
-			$base = $this->getRoutePrefix() . '/svg/';
-			$svg = false;
-			if (strpos($url, $base . 'core') === 0) {
-				$cleanUrl = substr($url, strlen($base.'core'));
-				$cleanUrl = substr($cleanUrl, 0, strpos($cleanUrl, '?'));
-				$parts = explode('/', $cleanUrl);
-				$color = array_pop($parts);
-				$cleanUrl = implode('/', $parts);
-				$location = \OC::$SERVERROOT . '/core/img/' . $cleanUrl . '.svg';
-				$svg = file_get_contents($location);
-			} elseif (strpos($url, $base) === 0) {
-				$cleanUrl = substr($url, strlen($base));
-				$cleanUrl = substr($cleanUrl, 0, strpos($cleanUrl, '?'));
-				$parts = explode('/', $cleanUrl);
-				$app = array_shift($parts);
-				$color = array_pop($parts);
-				$cleanUrl = implode('/', $parts);
-				$location = \OC_App::getAppPath($app) . '/img/' . $cleanUrl . '.svg';
-				if ($app === 'settings') {
-					$location = \OC::$SERVERROOT . '/settings/img/' . $cleanUrl . '.svg';
-				}
-				$svg = file_get_contents($location);
-			}
+			$list .= "--$icon: url('$url');";
+			list($location,$color) = $this->parseUrl($url);
+			$svg = file_get_contents($location);
 			if ($svg === false) {
 				$this->logger->debug('Failed to get icon file ' . $location);
 				$data .= "--$icon: url('$url');";
 				continue;
 			}
-			// TODO: Copied from SvgController (we should put this into a separate method so the controller can use it as well)
-			// add fill (fill is not present on black elements)
-			$fillRe = '/<((circle|rect|path)((?!fill)[a-z0-9 =".\-#():;])+)\/>/mi';
-			$svg = preg_replace($fillRe, '<$1 fill="#' . $color . '"/>', $svg);
-
-			// replace any fill or stroke colors
-			$svg = preg_replace('/stroke="#([a-z0-9]{3,6})"/mi', 'stroke="#' . $color . '"', $svg);
-			$svg = preg_replace('/fill="#([a-z0-9]{3,6})"/mi', 'fill="#' . $color . '"', $svg);
+			$encode = base64_encode($this->colorizeSvg($svg, $color));
+			$data .= '--' . $icon . ': url(data:image/svg+xml;base64,' . $encode . ');';
+		}
 
-			$encode = base64_encode($svg);
-			$data .= "--$icon: url(data:image/svg+xml;base64,$encode);";
+		if (\strlen($data) > 0 && \strlen($list) > 0) {
+			$data = ":root {\n$data\n}";
+			$cachedVarsCssFile->putContent($data);
+			$list = ":root {\n$list\n}";
+			$cachedFile->putContent($list);
 		}
 
-		if (strlen($data) > 0) {
-			if (!$cachedFile) {
-				$cachedFile = $this->folder->newFile($this->fileName);
-			}
+		return preg_replace($this->iconVarRE, '', $css);
+	}
 
-			$data = ":root {
-				$data
-			}";
-			$cachedFile->putContent($data);
+	/**
+	 * @param $url
+	 * @return array
+	 */
+	private function parseUrl($url): array {
+		$location = '';
+		$color = '';
+		$base = $this->getRoutePrefix() . '/svg/';
+		$cleanUrl = \substr($url, \strlen($base));
+		if (\strpos($url, $base . 'core') === 0) {
+			$cleanUrl = \substr($cleanUrl, \strlen('core'), \strpos($cleanUrl, '?')-\strlen('core'));
+			$parts = \explode('/', $cleanUrl);
+			$color = \array_pop($parts);
+			$cleanUrl = \implode('/', $parts);
+			$location = \OC::$SERVERROOT . '/core/img/' . $cleanUrl . '.svg';
+		} elseif (\strpos($url, $base) === 0) {
+			$cleanUrl = \substr($cleanUrl, 0, \strpos($cleanUrl, '?'));
+			$parts = \explode('/', $cleanUrl);
+			$app = \array_shift($parts);
+			$color = \array_pop($parts);
+			$cleanUrl = \implode('/', $parts);
+			$location = \OC_App::getAppPath($app) . '/img/' . $cleanUrl . '.svg';
+			if ($app === 'settings') {
+				$location = \OC::$SERVERROOT . '/settings/img/' . $cleanUrl . '.svg';
+			}
 		}
+		return [
+			$location,
+			$color
+		];
+	}
 
-		return preg_replace($this->iconVarRE, '', $css);
+	/**
+	 * @param $svg
+	 * @param $color
+	 * @return string
+	 */
+	public function colorizeSvg($svg, $color): string {
+		// add fill (fill is not present on black elements)
+		$fillRe = '/<((circle|rect|path)((?!fill)[a-z0-9 =".\-#():;])+)\/>/mi';
+		$svg = preg_replace($fillRe, '<$1 fill="#' . $color . '"/>', $svg);
+
+		// replace any fill or stroke colors
+		$svg = preg_replace('/stroke="#([a-z0-9]{3,6})"/mi', 'stroke="#' . $color . '"', $svg);
+		$svg = preg_replace('/fill="#([a-z0-9]{3,6})"/mi', 'fill="#' . $color . '"', $svg);
+		return $svg;
 	}
 
 	private function getRoutePrefix() {
@@ -177,6 +201,18 @@ class IconsCacher {
 		}
 	}
 
+	/**
+	 * Get icon-vars list template
+	 * @return ISimpleFile|boolean
+	 */
+	public function getCachedList() {
+		try {
+			return $this->folder->getFile($this->iconList);
+		} catch (NotFoundException $e) {
+			return false;
+		}
+	}
+
 	public function injectCss() {
 		// Only inject once
 		foreach (\OC_Util::$headers as $header) {

tests/Core/Controller/SvgControllerTest.php

@@ -26,6 +26,7 @@ namespace Tests\Core\Controller;
 
 use OC\AppFramework\Http;
 use OC\Core\Controller\SvgController;
+use OC\Template\IconsCacher;
 use OCP\App\IAppManager;
 use OCP\AppFramework\Utility\ITimeFactory;
 use OCP\IRequest;
@@ -89,7 +90,8 @@ class SvgControllerTest extends TestCase {
 		$request = $this->getMockBuilder(IRequest::class)->getMock();
 		$timeFactory = $this->getMockBuilder(ITimeFactory::class)->getMock();
 		$appManager = $this->getMockBuilder(IAppManager::class)->getMock();
-		$this->svgController = new SvgController('core', $request, $timeFactory, $appManager);
+		$iconsCacher = $this->getMockBuilder(IconsCacher::class)->disableOriginalConstructor()->setMethods(['__construct'])->getMock();
+		$this->svgController = new SvgController('core', $request, $timeFactory, $appManager, $iconsCacher);
 	}
 
 	/**

tests/lib/Template/IconsCacherTest.php

@@ -104,7 +104,7 @@ class IconsCacherTest extends \Test\TestCase {
 	public function testSetIconsFromValidCss() {
 		$css = "
 			icon.test {
-				--icon-test: url('/svg/core/actions/add/000?v=1');
+				--icon-test: url('/index.php/svg/core/actions/add/000?v=1');
 				background-image: var(--icon-test);
 			}
 		";
@@ -116,10 +116,10 @@ class IconsCacherTest extends \Test\TestCase {
 		";
 
 		$iconsFile = $this->createMock(ISimpleFile::class);
-		$this->folder->expects($this->once())
+		$this->folder->expects($this->exactly(2))
 			->method('getFile')
 			->willReturn($iconsFile);
-		
+
 		$actual = $this->iconsCacher->setIconsCss($css);
 		$this->assertEquals($expected, $actual);
 	}
@@ -127,7 +127,7 @@ class IconsCacherTest extends \Test\TestCase {
 	public function testSetIconsFromValidCssMultipleTimes() {
 		$css = "
 			icon.test {
-				--icon-test: url('/svg/core/actions/add/000?v=1');
+				--icon-test: url('/index.php/svg/core/actions/add/000?v=1');
 				background-image: var(--icon-test);
 			}
 		";
@@ -139,14 +139,14 @@ class IconsCacherTest extends \Test\TestCase {
 		";
 
 		$iconsFile = $this->createMock(ISimpleFile::class);
-		$this->folder->expects($this->exactly(3))
+		$this->folder->expects($this->exactly(6))
 			->method('getFile')
 			->willReturn($iconsFile);
-		
+
 		$actual = $this->iconsCacher->setIconsCss($css);
 		$actual = $this->iconsCacher->setIconsCss($actual);
 		$actual = $this->iconsCacher->setIconsCss($actual);
 		$this->assertEquals($expected, $actual);
 	}
 
-}
+}