Uploaded image for project: 'Moodle'
  1. Moodle
  2. MDL-43082

SVG Icons are displayed in IE8 compatibility view in IE9

    Details

    • Testing Instructions:
      Hide

      (difficulty: easy, requires IE 9, IE 10 and IE 11)

      1. Open IE 9;
      2. Press <F12>: click on the Network tab and press Start capturing;
      3. Browse the home page of Moodle using e.g. the standard theme;
      4. Check that the SVG-served icons are served by looking at the Type column in the Network tab and by zooming in (at least 350%, use <CTRL +>) the Navigation block to look for no blurring with the SVG-served icons;
      5. Press <F12>: hit Browser Mode to select the Compatibility View and hit the same page by pressing <CTRL F5> to reload (and refresh) the home page. Check for the blurring of the icons, previously served as SVG, now served as PNG: they will be blurred but visible. No image/svg+xml must appear in the Type column;
      6. Repeat the same steps for IE 10 and IE 11: the SVG-served icons must be present in both modes, native and Compatibility View.
      Show
      (difficulty: easy, requires IE 9, IE 10 and IE 11) Open IE 9; Press <F12>: click on the Network tab and press Start capturing ; Browse the home page of Moodle using e.g. the standard theme; Check that the SVG-served icons are served by looking at the Type column in the Network tab and by zooming in (at least 350%, use <CTRL +>) the Navigation block to look for no blurring with the SVG-served icons; Press <F12>: hit Browser Mode to select the Compatibility View and hit the same page by pressing <CTRL F5> to reload (and refresh) the home page. Check for the blurring of the icons, previously served as SVG, now served as PNG: they will be blurred but visible. No image/svg+xml must appear in the Type column; Repeat the same steps for IE 10 and IE 11: the SVG-served icons must be present in both modes, native and Compatibility View .
    • Affected Branches:
      MOODLE_24_STABLE, MOODLE_25_STABLE, MOODLE_26_STABLE
    • Fixed Branches:
      MOODLE_24_STABLE, MOODLE_25_STABLE, MOODLE_26_STABLE, MOODLE_27_STABLE
    • Pull from Repository:
    • Pull Master Branch:
      m27_MDL-43082_IE9_Compat_View_SVG_Say_No

      Description

      As reported on the forums: https://moodle.org/mod/forum/discuss.php?d=223136

      The SVG icons are served in compatibility view, but are not displayed.

        Gliffy Diagrams

          Issue Links

            Activity

            Hide
            poltawski Dan Poltawski added a comment -

            Actually, i'm trying to reproduce this and can't.

            Amy Groshek can you help?

            What I tried:

            1/ Go to Moodle in IE10
            2/ Open developer tools
            3/ Switch to IE8 mode and Document mode quirks.

            Show
            poltawski Dan Poltawski added a comment - Actually, i'm trying to reproduce this and can't. Amy Groshek can you help? What I tried: 1/ Go to Moodle in IE10 2/ Open developer tools 3/ Switch to IE8 mode and Document mode quirks.
            Hide
            salvetore Michael de Raadt added a comment -

            I wasn't able to replicate this in IE8 itself, with compatability view on or off.

            Show
            salvetore Michael de Raadt added a comment - I wasn't able to replicate this in IE8 itself, with compatability view on or off.
            Hide
            poltawski Dan Poltawski added a comment -

            It is only in IE8 compatibility mode. Not in IE8.

            Show
            poltawski Dan Poltawski added a comment - It is only in IE8 compatibility mode. Not in IE8.
            Hide
            matteo Matteo Scaramuccia added a comment -

            Amy Groshek added more details in https://moodle.org/mod/forum/discuss.php?d=223136#p1061130 - she had tracker login issues.
            I've replicated the issue using IE 9 running under Compatibility View and the fix is adding more conditions on the "IE branch" in core_useragent::supports_svg(): the fix presented in the community is slightly wrong (evening calls for sleep not code ).
            I'm now trying IE 10 to see if the issue is linked to Compatibility View or to IE9 under Compatibility View, to propose a PR.

            Show
            matteo Matteo Scaramuccia added a comment - Amy Groshek added more details in https://moodle.org/mod/forum/discuss.php?d=223136#p1061130 - she had tracker login issues. I've replicated the issue using IE 9 running under Compatibility View and the fix is adding more conditions on the "IE branch" in core_useragent::supports_svg() : the fix presented in the community is slightly wrong (evening calls for sleep not code ). I'm now trying IE 10 to see if the issue is linked to Compatibility View or to IE9 under Compatibility View , to propose a PR.
            Hide
            matteo Matteo Scaramuccia added a comment -

            IE 10 seems not to be affected, even when running in Compatibility View.

            Show
            matteo Matteo Scaramuccia added a comment - IE 10 seems not to be affected, even when running in Compatibility View .
            Hide
            matteo Matteo Scaramuccia added a comment -

            Hi Dan,
            could you peer review my PR? If OK, I'll backport it to 2.4/2.5 using the same logic but different code (no core_useragent::check_ie_compatibility_view()).

            TIA,
            Matteo

            Show
            matteo Matteo Scaramuccia added a comment - Hi Dan, could you peer review my PR? If OK, I'll backport it to 2.4/2.5 using the same logic but different code ( no core_useragent::check_ie_compatibility_view() ). TIA, Matteo
            Hide
            poltawski Dan Poltawski added a comment -

            Hi Matteo,

            While the fix seems fine, I have an aversion to long nested statements in if branches (they quickly can get out of control and the source of bugs).

            My preference would be to change that IF branch to check if its IE, then do another set of if statements to set the value of svg. I think it'd be easier to understand that way.

            Show
            poltawski Dan Poltawski added a comment - Hi Matteo, While the fix seems fine, I have an aversion to long nested statements in if branches (they quickly can get out of control and the source of bugs). My preference would be to change that IF branch to check if its IE, then do another set of if statements to set the value of svg. I think it'd be easier to understand that way.
            Hide
            matteo Matteo Scaramuccia added a comment -

            OK, will do on my evening, mostly out of this integration week but for sure ready for the next.

            Matteo

            Show
            matteo Matteo Scaramuccia added a comment - OK, will do on my evening, mostly out of this integration week but for sure ready for the next. Matteo
            Hide
            matteo Matteo Scaramuccia added a comment -

            Hi Dan,
            please, share your thoughts about my PRs, 2.6+ and 2.5-.

            TIA,
            Matteo

            Show
            matteo Matteo Scaramuccia added a comment - Hi Dan, please, share your thoughts about my PRs, 2.6+ and 2.5- . TIA, Matteo
            Hide
            matteo Matteo Scaramuccia added a comment -

            Fixed core_theme_config_testcase() in 2.6+ and aligned the inline comments of $useragents items for theme_config_testcase() in 2.5-.

            Sorry Dan for these two issues, now the PRs are available for your peer review.
            Matteo

            Show
            matteo Matteo Scaramuccia added a comment - Fixed core_theme_config_testcase() in 2.6+ and aligned the inline comments of $useragents items for theme_config_testcase() in 2.5- . Sorry Dan for these two issues, now the PRs are available for your peer review. Matteo
            Hide
            poltawski Dan Poltawski added a comment -

            Thanks Matteo, I was thinking of a different way of doing the if which confused me at first, but this seems good to me.

            Show
            poltawski Dan Poltawski added a comment - Thanks Matteo, I was thinking of a different way of doing the if which confused me at first, but this seems good to me.
            Hide
            agroshek Amy Groshek added a comment -

            Whoohoo, I'm back in the tracker! Matteo Scaramuccia, thanks so much!

            Show
            agroshek Amy Groshek added a comment - Whoohoo, I'm back in the tracker! Matteo Scaramuccia , thanks so much!
            Hide
            poltawski Dan Poltawski added a comment -

            The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week.

            TIA and ciao

            Show
            poltawski Dan Poltawski added a comment - The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week. TIA and ciao
            Hide
            matteo Matteo Scaramuccia added a comment -

            Rebased.

            Show
            matteo Matteo Scaramuccia added a comment - Rebased.
            Hide
            marina Marina Glancy added a comment -

            Thanks Matteo, integrated in 2.4, 2.5, 2.6 and master

            it's so great to see such a good test coverage!

            Show
            marina Marina Glancy added a comment - Thanks Matteo, integrated in 2.4, 2.5, 2.6 and master it's so great to see such a good test coverage!
            Hide
            rwijaya Rossiani Wijaya added a comment -

            Hi Matteo,

            In IE11, IE10 and IE9 with 2.4 branch: with or without compatibility view, the icon is displayed with png icon.

            Additional, I also tested this for IE8 and noting the result here.
            In IE8 with 2.4, 2.5, 2.6 and master: the icon is displayed with png icon with or without compability view.

            I'm failing this because of the patch is not working for 2.4.

            Show
            rwijaya Rossiani Wijaya added a comment - Hi Matteo, In IE11, IE10 and IE9 with 2.4 branch: with or without compatibility view, the icon is displayed with png icon. Additional, I also tested this for IE8 and noting the result here. In IE8 with 2.4, 2.5, 2.6 and master: the icon is displayed with png icon with or without compability view. I'm failing this because of the patch is not working for 2.4.
            Hide
            matteo Matteo Scaramuccia added a comment -

            Hi Rossiani,
            will look at it this evening, in a few hours.

            TNX,
            Matteo

            Show
            matteo Matteo Scaramuccia added a comment - Hi Rossiani, will look at it this evening, in a few hours. TNX, Matteo
            Hide
            matteo Matteo Scaramuccia added a comment -

            Hi Rossiani,
            just performed some tests in a brand new 2.4.7+ (Build: 20131205). The unit tests - now there is a difference between IE9 Native and Compat View - are running fine:

            # vendor/bin/phpunit theme_config_testcase lib/tests/outputlib_test.php
            PHPUnit 3.7.28 by Sebastian Bergmann.
             
            Configuration read from /path/to/moodle-24/phpunit.xml
             
            .
             
            Time: 60 ms, Memory: 36.25Mb
             
            OK (1 test, 27 assertions)

            and the Network Capturing of the home page - here when logged in as admin - shows that SVG are served when in Native while PNG when in Compat View, as expected after applying the PR:

            Did you find a specific icon or all the icons to be served always as PNG? Some icons will always be served as PNG due to the lack of the SVG alternative.

            Show
            matteo Matteo Scaramuccia added a comment - Hi Rossiani, just performed some tests in a brand new 2.4.7+ (Build: 20131205) . The unit tests - now there is a difference between IE9 Native and Compat View - are running fine: # vendor/bin/phpunit theme_config_testcase lib/tests/outputlib_test.php PHPUnit 3.7.28 by Sebastian Bergmann.   Configuration read from /path/to/moodle-24/phpunit.xml   .   Time: 60 ms, Memory: 36.25Mb   OK (1 test, 27 assertions) and the Network Capturing of the home page - here when logged in as admin - shows that SVG are served when in Native while PNG when in Compat View , as expected after applying the PR: Did you find a specific icon or all the icons to be served always as PNG ? Some icons will always be served as PNG due to the lack of the SVG alternative.
            Hide
            marina Marina Glancy added a comment -

            There is no difference in the code between 2.4 and 2.5, Rossie can you please make sure you have a failure?

            Show
            marina Marina Glancy added a comment - There is no difference in the code between 2.4 and 2.5, Rossie can you please make sure you have a failure?
            Hide
            rwijaya Rossiani Wijaya added a comment -

            Hi Matteo,

            I re-test this issue and checked the icon folder for expanded and collapsed icons. Both icons are available in svg and png.

            I also attached the network capturing screenshot. This can be seen in front page navigation block.

            Show
            rwijaya Rossiani Wijaya added a comment - Hi Matteo, I re-test this issue and checked the icon folder for expanded and collapsed icons. Both icons are available in svg and png. I also attached the network capturing screenshot. This can be seen in front page navigation block.
            Hide
            marina Marina Glancy added a comment -

            Sending back to testing, there seem to be another bug involved. Rossi would you please create an issue for it?

            Show
            marina Marina Glancy added a comment - Sending back to testing, there seem to be another bug involved. Rossi would you please create an issue for it?
            Hide
            rwijaya Rossiani Wijaya added a comment -

            Apart from the bug in 2.4 for collapse and expanded icons, this is working as expected.

            I created MDL-43357 to fix the above bug.

            Passing this issue.

            Show
            rwijaya Rossiani Wijaya added a comment - Apart from the bug in 2.4 for collapse and expanded icons, this is working as expected. I created MDL-43357 to fix the above bug. Passing this issue.
            Hide
            samhemelryk Sam Hemelryk added a comment -

            Thanks for the code, its now upstream!

            Heres a fun trick to try in the spirit of Friday the 13th.
            I hear if you stand in front a mirror, alone, in the dark, and say "Oracle" three times Petr Skoka will appear in the mirror and you'll see him deleting the Oracle driver from Moodle.

            Show
            samhemelryk Sam Hemelryk added a comment - Thanks for the code, its now upstream! Heres a fun trick to try in the spirit of Friday the 13th. I hear if you stand in front a mirror, alone, in the dark, and say "Oracle" three times Petr Skoka will appear in the mirror and you'll see him deleting the Oracle driver from Moodle.

              People

              • Votes:
                0 Vote for this issue
                Watchers:
                6 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved:
                  Fix Release Date:
                  13/Jan/14