ui: Use inline SVG images for app icons for dark mode adaptation

- Use 'currentColor' as color in SVG images to make them adapt to light/dark
modes.

- Introduce a new {% icon %} template tag to read SVG files, attach attributes,
remove <?xml> header, and make all IDs inside unique.

- Use the {% icon %} template tag to inline SVGs in app page, home page,
app pages, and notifications.

- Relax the content security policy to allow inline styling with 'style='
attribute.

Tests:

- Allow the icons looks as before in the following pages: home page, apps page,
app specific page, and in notifications.

- Icons that are fully dark become white when theme is switched to dark mode.

- All the inlined SVG icons have a prefixed unique ID.

- W3C HTML validator shows no new errors.

Signed-off-by: Sunil Mohan Adapa <sunil@medhas.org>
Reviewed-by: James Valleroy <jvalleroy@mailbox.org>

plinth/middleware.py

@@ -230,7 +230,7 @@ CONTENT_SECURITY_POLICY = CSPDict({
     # scripts).
     'script-src': "'self'",
     # Allow inline CSS and CSS files from Freedombox itself.
-    'style-src': "'self'",
+    'style-src': "'self' 'unsafe-inline'",
     # Web worker sources are allowed only from FreedomBox itself (for
     # JSXC).
     'worker-src': "'self'",

plinth/templates/app-header.html

@@ -5,15 +5,16 @@
 {% load bootstrap %}
 {% load i18n %}
 {% load static %}
+{% load plinth_extras %}
 
 <header class="app-header {% if not app_info.icon_filename %} app-header-single-column {% endif %}">
 
   {% if app_info.icon_filename %}
 
     {% if app_info.app_id %}
-      <img src="{% static app_info.app_id %}/icons/{{ app_info.icon_filename }}.svg" alt="{{ app_info.name  }}"/>
+      {% icon app_info.app_id|add:'/icons/'|add:app_info.icon_filename|add:'.svg' %}
     {% else %}
-      <img src="{% static 'theme/icons/' %}{{ app_info.icon_filename }}.svg" alt="{{ app_info.name  }}"/>
+      {% icon app_info.icon_filename %}
     {% endif %}
 
   {% endif %}

plinth/templates/card.html

@@ -4,6 +4,7 @@
 
 {% load i18n %}
 {% load static %}
+{% load plinth_extras %}
 
 <div class="card">
   <a href="{{ item.url }}">
@@ -12,9 +13,9 @@
         <span class="fa {{ item.icon }}"></span>
       {% else %}
         {% if item.app_id %}
-          <img src="{% static item.app_id %}/icons/{{ item.icon }}.svg"/>
+          {% icon item.app_id|add:'/icons/'|add:item.icon|add:'.svg' class='card-icon' %}
         {% else %}
-          <img src="{% static 'theme/icons/' %}{{ item.icon }}.svg"/>
+          {% icon item.icon class='card-icon' %}
         {% endif %}
       {% endif %}
     </div>

plinth/templates/index.html

@@ -5,7 +5,7 @@
 
 {% load i18n %}
 {% load static %}
-
+{% load plinth_extras %}
 
 {% block body_class %}index-page{% endblock %}
 
@@ -66,9 +66,9 @@
                             <img src="{{ shortcut.icon }}"/>
                           {% else %}
                             {% if shortcut.app_id %}
-                              <img src="{% static shortcut.app_id %}/icons/{{ shortcut.icon }}.svg"/>
+                              {% icon shortcut.app_id|add:'/icons/'|add:shortcut.icon|add:'.svg' class='card-icon' %}
                             {% else %}
-                              <img src="{% static 'theme/icons/' %}{{ shortcut.icon }}.svg"/>
+                              {% icon shortcut.icon class='card-icon' %}
                             {% endif %}
                           {% endif %}
                         </div>

plinth/templates/notifications.html

@@ -6,6 +6,7 @@
 
 {% load i18n %}
 {% load static %}
+{% load plinth_extras %}
 
 {% if notifications %}
   <div id="notifications" class="notifications collapse no-no-js" hx-swap-oob="true">
@@ -27,13 +28,9 @@
                   <div class="app-icon fa {{ note.data.app_icon }}"></div>
                 {% elif note.data.app_icon_filename %}
                   {% if note.app_id %}
-                    <img src="{% static note.app_id %}/icons/{{ note.data.app_icon_filename }}.svg"
+                    {% icon note.app_id|add:'/icons/'|add:note.data.app_icon_filename|add:'.svg' alt=note.data.app_name class='notification-icon' %}
-                         alt="{{ note.data.app_name }}"
-                         class="notification-icon" />
                   {% else %}
-                    <img src="{% static 'theme/icons/' %}{{ note.data.app_icon_filename }}.svg"
+                    {% icon note.data.app_icon_filename alt=note.data.app_name class='notification-icon' %}
-                         alt="{{ note.data.app_name }}"
-                         class="notification-icon" />
                   {% endif %}
                 {% endif %}
                 {{ note.data.app_name }}

plinth/templatetags/plinth_extras.py

@@ -4,8 +4,10 @@ import os
 from urllib.parse import urlparse
 
 from django import template
+from django.utils.safestring import mark_safe
 
 from plinth import clients as clients_module
+from plinth import utils, web_server
 
 register = template.Library()
 
@@ -88,3 +90,36 @@ def clients_get_platforms(clients):
                 other.append(platform)
 
     return {'web': web, 'other': other}
+
+
+@register.simple_tag(name='icon')
+def icon(url: str, *args, **kwargs):
+    """Insert an SVG icon inline."""
+    icon_name = url.rpartition('/')[2].partition('.')[0]
+
+    def add_attributes(text: str) -> str:
+        """Append attributes to the <svg> elment."""
+        if 'class' not in kwargs:
+            kwargs['class'] = 'svg-icon'
+
+        kwargs['data-icon-name'] = icon_name
+        attributes = ' '.join(
+            (f'{key}="{value}"' for key, value in kwargs.items()))
+        text = text.replace('<svg', f'<svg {attributes} ', count=1)
+        text = text.replace('autoidmagic-', utils.random_string() + '-')
+        return text
+
+    if '/' not in url:
+        # Only icon name specified
+        url = f'theme/icons/{url}.svg'
+
+    path = web_server.resolve_static_path(url)
+    try:
+        icon_lines = path.read_text().splitlines()
+    except FileNotFoundError:
+        raise ValueError(f'Icon {url} not found.')
+    else:
+        # Skip the line with <?xml> header.
+        icon_text = add_attributes('\n'.join(icon_lines[1:]))
+
+    return mark_safe(icon_text)

plinth/tests/test_templatetags.py

@@ -3,6 +3,8 @@
 Test module for custom Django template tags.
 """
 
+from unittest.mock import patch
+
 from plinth.templatetags import plinth_extras
 
 
@@ -45,3 +47,59 @@ def test_highlighting():
         menu = plinth_extras.mark_active_menuitem(menu, check_path)
         _assert_active_url(menu, expected_active_path)
         assert _verify_active_menuitems(menu)
+
+
+@patch('plinth.web_server.resolve_static_path')
+def test_icon(resolve_static_path, tmp_path):
+    """Test that the icon tag works for basic usage."""
+    icon1 = tmp_path / 'icon1.svg'
+    icon1.write_text('<svg><cirlcle></circle></svg>')
+    resolve_static_path.return_value = icon1
+    return_value = plinth_extras.icon('icon1')
+    assert return_value == ('<svg class="svg-icon" data-icon-name="icon1" '
+                            '><cirlcle></circle></svg>')
+
+
+@patch('plinth.web_server.resolve_static_path')
+def test_icon_attributes(resolve_static_path, tmp_path):
+    """Test that the icon tag works with attributes."""
+    icon1 = tmp_path / 'icon1.svg'
+    icon1.write_text('<svg><cirlcle></circle></svg>')
+    resolve_static_path.return_value = icon1
+
+    attributes = {'class': 'test-class'}
+    return_value = plinth_extras.icon('icon1', **attributes)
+    assert return_value == ('<svg class="test-class" data-icon-name="icon1" '
+                            '><cirlcle></circle></svg>')
+
+    attributes = {'data-test': 'test-value'}
+    return_value = plinth_extras.icon('icon1', **attributes)
+    assert return_value == ('<svg data-test="test-value" class="svg-icon" '
+                            'data-icon-name="icon1" ><cirlcle></circle></svg>')
+
+
+@patch('plinth.utils.random_string')
+@patch('plinth.web_server.resolve_static_path')
+def test_icon_auto_ids(resolve_static_path, random_string, tmp_path):
+    """Test that the icon tag works for implementing automatic IDs."""
+    random_string.return_value = 'randomvalue'
+    icon2 = tmp_path / 'icon2.svg'
+    icon2.write_text('<svg><cirlcle id="autoidmagic-foo"></circle>'
+                     '<path id="autoidmagic-bar"></path></svg>')
+    resolve_static_path.return_value = icon2
+    return_value = plinth_extras.icon('icon2')
+    assert return_value == ('<svg class="svg-icon" data-icon-name="icon2" >'
+                            '<cirlcle id="randomvalue-foo"></circle>'
+                            '<path id="randomvalue-bar"></path></svg>')
+
+
+@patch('plinth.web_server.resolve_static_path')
+def test_icon_xml_stripping(resolve_static_path, tmp_path):
+    """Test that the icon tag strips the XML header."""
+    icon2 = tmp_path / 'icon2.svg'
+    icon2.write_text('<?xml version="1.0" encoding="utf-8">\n'
+                     '<svg><cirlcle></circle></svg>')
+    resolve_static_path.return_value = icon2
+    return_value = plinth_extras.icon('icon2')
+    assert return_value == ('<svg class="svg-icon" data-icon-name="icon2" >'
+                            '<cirlcle></circle></svg>')

plinth/tests/test_web_server.py

@@ -3,11 +3,12 @@
 Tests for CherryPy web server setup and its components.
 """
 
-from unittest.mock import call, patch
+import pathlib
+from unittest.mock import Mock, call, patch
 
 import pytest
 
-from plinth.web_server import StaticFiles
+from plinth.web_server import StaticFiles, resolve_static_path
 
 
 @pytest.fixture(autouse=True)
@@ -61,3 +62,40 @@ def test_static_files_mount(mount, load_cfg):
             })
     ]
     mount.assert_has_calls(calls)
+
+
+@patch('sys.modules')
+@patch('plinth.app.App.list')
+def test_resolve_static_path(app_list, sys_modules, tmp_path):
+    """Test that resolving a static path works as expected."""
+    app_list.return_value = []
+    expected_path = (pathlib.Path(__file__).parent.parent.parent /
+                     'static/theme/icons/test.svg')
+    assert resolve_static_path('theme/icons/test.svg') == expected_path
+
+    app = Mock()
+    app.app_id = 'test-app'
+    app.__module__ = 'test-module'
+    app_list.return_value = [app]
+    expected_path = (pathlib.Path(__file__).parent.parent.parent /
+                     'static/theme/icons/test.svg')
+
+    sys_modules.__getitem__.side_effect = {}.__getitem__
+    assert resolve_static_path('theme/icons/test.svg') == expected_path
+    with pytest.raises(ValueError, match='Module for app not loaded'):
+        resolve_static_path('test-app/test.svg')
+
+    module = Mock()
+    sys_modules.__getitem__.side_effect = {'test-module': module}.__getitem__
+    with pytest.raises(ValueError,
+                       match='Module file for app could not be found'):
+        resolve_static_path('test-app/test.svg')
+
+    module.__file__ = tmp_path / 'test-module.py'
+    with pytest.raises(ValueError,
+                       match='No static directory available for app test-app'):
+        resolve_static_path('test-app/test.svg')
+
+    (tmp_path / 'static').mkdir()
+    assert resolve_static_path(
+        'test-app/test.svg') == tmp_path / 'static/test.svg'

plinth/web_server.py

@@ -185,3 +185,29 @@ class StaticFiles(app_module.FollowerComponent):
             for web_path, file_path in self.directory_map.items():
                 web_path = '%s%s' % (cfg.server_dir, web_path)
                 _mount_static_directory(file_path, web_path)
+
+
+def resolve_static_path(url: str) -> pathlib.Path:
+    """Convert a URL for static file into a file path."""
+    url_parts = url.split('/')
+    for app in app_module.App.list():
+        if url_parts[0] != app.app_id:
+            continue
+
+        try:
+            module = sys.modules[app.__module__]
+        except KeyError:
+            raise ValueError('Module for app not loaded')
+
+        if not hasattr(module, '__file__') or not module.__file__:
+            raise ValueError('Module file for app could not be found')
+
+        module_path = pathlib.Path(module.__file__).parent
+        static_dir = module_path / 'static'
+        if not static_dir.is_dir():
+            raise ValueError(
+                f'No static directory available for app {app.app_id}')
+
+        return static_dir / '/'.join(url_parts[1:])
+
+    return pathlib.Path(__file__).parent.parent / 'static' / url

static/themes/default/css/main.css

@@ -354,6 +354,16 @@ footer {
     padding-top: 20rem;
 }
 
+/*
+ * Inline SVG Icons
+ */
+.svg-icon {
+    width: auto;
+    height: 0.875em;
+    vertical-align: sub;
+    margin: 0.125em 0.25em;
+}
+
 /*
  * Bootstrap extensions
  */
@@ -587,7 +597,7 @@ footer {
 }
 
 .card-icon span,
-.card-icon img {
+.card-icon svg {
     width: 6.25rem;
     height: 6.25rem;
     font-size: 5rem;
@@ -816,9 +826,10 @@ input[type='submit'].running-status-button {
     margin-bottom: 1.25rem;
 }
 
-.app-header > img {
+.app-header > svg {
     margin: 0 auto;
     width: 100%;
+    height: auto;
 }
 
 section.app-description {
@@ -867,7 +878,7 @@ section.app-description {
         flex-flow: column;
     }
 
-    .app-header img {
+    .app-header svg {
         width: 9.375rem;
         height: 9.375rem;
         margin-top: 0;
@@ -974,7 +985,7 @@ section.app-description {
     font-weight: bold;
 }
 
-img.notification-icon {
+.notification-icon {
     display: inline-block;
     width: 0.875rem;
     height: 0.875rem;