Use SVG images and provide backward compatibility

We don't have any immediate plans to do this, but if a person were to implement this, the place to start would be looking at the Theme class in lib/web.php. Currently images are served up with the $THEME->get_url() method, which abstracts out the specific theme to allow for theme inheritance. We'd need to add a second method alongside that abstracts out the file suffix for image files.

See https://wiki.mahara.org/index.php/Customising/Themes/1.10#Core_assets for details on how it currently works.

Cacheing it is a separate issue. Our theme system currently doesn't do any cacheing at all. Every time it's looking for a particular file, it hits the filesystem to find it. I haven't run any tests to see whether this causes us any slowdown, though. I suppose an easy way to test it would be to modify $THEME->get_url() so that it's hardcoded to use the "raw" theme instead of checking for the existence of files.

And the third issue is browser detection. We currently have a library that detects whether you're using a phone or table, but we don't do any browser sniffing beyond that. In fact, in the Mahara README we limit Mahara support to the latest versions of Firefox & Chrome, and the last three releases of IE (currently IE9, 10, and 11). All of those support SVG files: http://caniuse.com/#feat=svg

So, we wouldn't be adding any code to Mahara core to check for browser support for SVGs. But if we did implement the file-format-abstracting system described above, we could add a local/lib.php hook in lib/web.php to allow sites have to support IE8 to customize the process of deciding which file format to use.

I agree with all your comments.

We already coded a method in Theme class in the lib/web.php file. It doesn't have caching neither have support for IE8 and below but I think it is simple and do the job:

    /**
     * Adds the URL of an image with the extension svg if it exists, png otherwise.
     *
     * @param string $filename The name of the file without the extension
     * @param string $plugindirectory The plugin directory
     * @return string The image URL with the correct file extension.
     */
    public function get_image_url_with_extension($filename, $plugindirectory = '') {
        $ext = '.png';
        if (file_exists($this->_get_path($filename . '.svg', false, $plugindirectory, get_config('docroot')))) {
            $ext = '.svg';
        }
        return $this->get_url($filename . $ext, false, $plugindirectory);
    }

Note that it don't support the multi-files array possible with the get_url param (second parameter) but I dont think its a problem.

I'm new to mahara and I don't know yet how to share with you but I may create a Git branch if you want. We could maybe create the same for the path, something like get_image_path_with_extension.

It should be great to have the same thing in javascript. That could be another task too.

Hi Gilles,

If you're interested in contributing your code back upstream, there are some instructions on the mahara.org wiki: https://wiki.mahara.org/index.php/Developer_Area/Contributing_Code

We use the gerrit code review system, which is based on git. If you can't get that working, I'm also willing to just take a look at a patch file or a github branch. :)

I suppose what we would want, is a new method, $THEME->get_image_url($imagename). This would be equivalent to a call to $THEME->get_url("images/{$imagename}.png"), except that it would check for multiple file extension types. And then we'd want to go through the existing code and find all the current places that are calling $THEME->get_url() for images and replace them with calls to $THEME->get_image_url().

This would multiply the number of filesystem checks to find an image file, so it indeed might be good to do this in conjunction with some cacheing to cut down on that.

Cheers,
Aaron

Hi,

We choose to work on this task on our current sprint and I see you just start to work on it too.
If I can help you to fix this task, feel free to ask me.

Thanks,
Gilles-Philippe

In addition to my previous comment, if no one has started this task, I will be happy to do so.
Just tell me where you are with this.

Cheers,
Gilles-Philippe

Hi Gilles,

Nope, nobody here at Mahara HQ is working on it. Kristina set it to "In Progress" because she thought that you were already working on it. :)

I've assigned the bug to you for clarification.

Cheers,
Aaron

Patch for "master" branch: https://reviews.mahara.org/4449

Here's the patch for this improvment.

What is covered:
* Appel THEME->get_url() becomes THEME->get_image_url()
* Dans les templates, theme_url becomes theme_image_url

What is not covered :

* Create method call get_image_url in javascript
* Allow to use SVG in thumbnails (thumbs.php Files)
* Create a method to relative links (used $ smarty-> assign ('maharalogofilename', 'images / Site-logo-small .png')).
* Fix for atom.php files and viewlayout.php to get_config size ('wwwroot'). 'Theme / raw / static / images / logo.png Site';
* Manage License images svg (license.php)
* Manage themepaths () function in lib / web.php

Also, I have not implemented cache because it seemed too expensive.
A general cache solution like memcached seems a better solution.
However, this should not add significant load because the use of the svg file is disabled bu default.
Only theme using will check for this format.

To test, make sure all the lines of code have been replaced.
Otherwise, run the following load test:
phpunit ThemeTest htdocs/lib/tests/phpunit/ThemeTest.php

Finnally, the documentation may be updated once this task is accepted.

To be peer-reviewed.

Hello Gilles-Philippe,

Thank you for the additional information. In terms of testing the patch on the front-end, what would need to be done? Would we just need to check that the current images, e.g. the block images, the license images are still displayed correctly? The only image you seem to have replaced is the site logo. Would that be correct?

Thanks
Kristina

Hi Kristina,

By default, There is no SVG used at all.
I have added a config, disabled by default, that allow the use of SVG in a theme.
This config must be enabled in the themeconfig.php like this to allow the use of SVG images :
$theme->usesvg = true;

The site-logo.svg was only added as an example and to be used in the phpunit tests.

I coded this functionality in this way in order not to degrade performance when calling the images and to avoid unnecessary search for SVG files if the theme simply do not use and to allow a iterative development that will allow you to make the conversion or the addition of SVG icons.

In short, if you want to manually test the use of SVG files, simply add "$theme->usesvg = true;" in the themeconfig.php file of the raw theme or in any theme of your choice. One enabled, you should see site-logo.svg in the header of the page.

Hello Gilles-Philippe,

Thank you for the additional info. That'll help for the review.

Cheers
Kristina

Reviewed: https://reviews.mahara.org/4449
Committed: http://gitorious.org/mahara/mahara/commit/cf4f8bd63e603542f43e79816270c038594fd0cd
Submitter: Son Nguyen (<email address hidden>)
Branch: master

commit cf4f8bd63e603542f43e79816270c038594fd0cd
Author: Gilles-Philippe Leblanc <email address hidden>
Date: Tue Mar 17 13:09:22 2015 -0400

Use SVG images and provide backward compatibility (Bug #1416890)

Change-Id: I724a844fc9e4fbae9e58834082279b4f3f21995c

Patch for "15.04_STABLE" branch: https://reviews.mahara.org/4529

Patch for "master" branch: https://reviews.mahara.org/4840

Reviewed: https://reviews.mahara.org/4840
Committed: https://git.nzoss.org.nz/mahara/mahara/commit/22f59dbbe97b7629156d9d2fe60d4786c47cf12e
Submitter: Aaron Wells (<email address hidden>)
Branch: master

commit 22f59dbbe97b7629156d9d2fe60d4786c47cf12e
Author: Robert Lyon <email address hidden>
Date: Tue Jun 16 11:51:40 2015 +1200

Regression: Adding back in the site-logo.svg file (Bug #1416890)

And fixing the phpunit tests relating to this change

Change-Id: I8ed7456fdde0427b8f921a64b44f00e1e8274df0
Signed-off-by: Robert Lyon <email address hidden>