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

The hide/show eye icon on labels is incorrect when AJAX is enabled

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: 1.8, 1.8.1, 1.9.2
    • Fix Version/s: 1.8.9, 1.9.5
    • Component/s: JavaScript
    • Labels:
      None
    • Environment:
      Any
    • Affected Branches:
      MOODLE_18_STABLE, MOODLE_19_STABLE
    • Fixed Branches:
      MOODLE_18_STABLE, MOODLE_19_STABLE

      Description

      The hide/show eye icon on labels always shows as open if AJAX is enabled.

      To reproduce:

      Make sure AJAX is enabled.
      Turn editing on.
      Create a label.
      Hide the label by clicking the eye icon, or by going to the update label screen and setting the visible property to hide.
      Navigate back to the course page (or refresh it) - the eye is now open again.

      Note that the label is actually hidden from students - it is just that the eye icon is incorrect, leading teachers to believe that the label is not hidden when it really is.

        Gliffy Diagrams

          Attachments

            Activity

            Hide
            adamann2 Ann Adamcik added a comment -

            Other resources are links, with 'class="dimmed"' if they are hidden. Labels are spans, with 'class="dimmed_text"' if they are hidden. The AJAX code only checks links when determining whether or not the eye icon should be closed. It needs to check for dimmed_text spans as well. Here's a patch for 1.8+ and 1.9 -

            RCS file: /cvsroot/moodle/moodle/lib/ajax/section_classes.js,v
            retrieving revision 1.32.2.2
            diff -c -r1.32.2.2 section_classes.js

                • section_classes.js 27 Aug 2007 17:27:00 -0000 1.32.2.2
                • section_classes.js 6 Nov 2007 14:21:19 -0000
                  ***************
                • 516,522 ****
                  this.id = this.getEl().id.replace(/module-/i, '');

            this.hidden = false;
            ! if (YAHOO.util.Dom.hasClass(this.getEl().getElementsByTagName('a')[0], 'dimmed'))

            { this.hidden = true; }
            this.hiddenStored = null;
            — 516,522 ----
            this.id = this.getEl().id.replace(/module-/i, '');

            this.hidden = false;
            ! if (YAHOO.util.Dom.hasClass(this.getEl().getElementsByTagName('a')[0], 'dimmed') || YAHOO.util.Dom.hasClass(this.getEl().getElementsByTagName('span')[0], 'dimmed_text')) { this.hidden = true; }

            this.hiddenStored = null;

            Show
            adamann2 Ann Adamcik added a comment - Other resources are links, with 'class="dimmed"' if they are hidden. Labels are spans, with 'class="dimmed_text"' if they are hidden. The AJAX code only checks links when determining whether or not the eye icon should be closed. It needs to check for dimmed_text spans as well. Here's a patch for 1.8+ and 1.9 - RCS file: /cvsroot/moodle/moodle/lib/ajax/section_classes.js,v retrieving revision 1.32.2.2 diff -c -r1.32.2.2 section_classes.js section_classes.js 27 Aug 2007 17:27:00 -0000 1.32.2.2 section_classes.js 6 Nov 2007 14:21:19 -0000 *************** 516,522 **** this.id = this.getEl().id.replace(/module-/i, ''); this.hidden = false; ! if (YAHOO.util.Dom.hasClass(this.getEl().getElementsByTagName('a') [0] , 'dimmed')) { this.hidden = true; } this.hiddenStored = null; — 516,522 ---- this.id = this.getEl().id.replace(/module-/i, ''); this.hidden = false; ! if (YAHOO.util.Dom.hasClass(this.getEl().getElementsByTagName('a') [0] , 'dimmed') || YAHOO.util.Dom.hasClass(this.getEl().getElementsByTagName('span') [0] , 'dimmed_text')) { this.hidden = true; } this.hiddenStored = null;
            Hide
            hillig Markus Hillig added a comment -

            Works perfectly in Moodle 1.8.3+. Thanks for the fix, Ann.

            Show
            hillig Markus Hillig added a comment - Works perfectly in Moodle 1.8.3+. Thanks for the fix, Ann.
            Hide
            dougiamas Martin Dougiamas added a comment -

            Nicolas can you review and merge this please?

            Show
            dougiamas Martin Dougiamas added a comment - Nicolas can you review and merge this please?
            Hide
            thornleyk Kristian Thornley added a comment -

            Looks like this fix works partially on Moodle 1.9.2.+

            Problem now is the style changes but this is not reflected in the DOM

            proposed solution:

            change <<Moodle Root>>/course/lib.php and make labels have the class dimmed if they are hidden and always wrapped in a div

            replace lines 1350 - 1356 with
            echo "<div ";
            if (!$mod->visible)

            { echo "class='dimmed' "; }

            echo ">";
            echo format_text($extra, FORMAT_HTML, $labelformatoptions);
            echo "</div>";

            then modify css for theme so that div's with dimmed are greyed out e.g.

            a.dimmed:link,
            a.dimmed:visited,
            div.dimmed {
            color:#AAAAAA;
            }

            then modify and insert following code round about line 537 of <<Moodle Root>>/lib/ajax/section_classes.js (look for this line this.linkContainer = this.getEl().getElementsByTagName('a')[0]

            if (YAHOO.util.Dom.hasClass(this.getEl().getElementsByTagName('div')[0], 'dimmed'))

            { this.hidden = true; }

            if(this.linkContainer.parentNode.className == 'commands'){
            this.linkContainer = this.getEl().getElementsByTagName('div')[0];
            }

            Show
            thornleyk Kristian Thornley added a comment - Looks like this fix works partially on Moodle 1.9.2.+ Problem now is the style changes but this is not reflected in the DOM proposed solution: change <<Moodle Root>>/course/lib.php and make labels have the class dimmed if they are hidden and always wrapped in a div replace lines 1350 - 1356 with echo "<div "; if (!$mod->visible) { echo "class='dimmed' "; } echo ">"; echo format_text($extra, FORMAT_HTML, $labelformatoptions); echo "</div>"; then modify css for theme so that div's with dimmed are greyed out e.g. a.dimmed:link, a.dimmed:visited, div.dimmed { color:#AAAAAA; } then modify and insert following code round about line 537 of <<Moodle Root>>/lib/ajax/section_classes.js (look for this line this.linkContainer = this.getEl().getElementsByTagName('a') [0] if (YAHOO.util.Dom.hasClass(this.getEl().getElementsByTagName('div') [0] , 'dimmed')) { this.hidden = true; } if(this.linkContainer.parentNode.className == 'commands'){ this.linkContainer = this.getEl().getElementsByTagName('div') [0] ; }
            Hide
            bobpuffer Bob Puffer added a comment -

            This is still an issue in 1.92+ (20080711) and I'd definitely consider it major. Not only does it display the incorrect state of the "eye" but it disallows instructors from making a resource visible once it takes on an invisible state with the eye opened. A patch would be great.

            Show
            bobpuffer Bob Puffer added a comment - This is still an issue in 1.92+ (20080711) and I'd definitely consider it major. Not only does it display the incorrect state of the "eye" but it disallows instructors from making a resource visible once it takes on an invisible state with the eye opened. A patch would be great.
            Hide
            hillig Markus Hillig added a comment -

            For what it's worth it: for Moodle 1.9.2+ I had to adapt Ann's above patch from 07/Nov/07 since now dimmed text gets enclosed between div tags on no longer between span tags. This is what I did. For me it works so far.

            [root@ecampus html]# diff -c moodle/lib/ajax/section_classes.js.dist moodle/lib/ajax/section_classes.js

                • moodle/lib/ajax/section_classes.js.dist 2008-07-19 02:17:39.000000000 +0200
                • moodle/lib/ajax/section_classes.js 2008-09-26 16:14:42.000000000 +0200
                  ***************
                • 526,532 ****
                  this.id = this.getEl().id.replace(/module-/i, '');

            this.hidden = false;
            ! if (YAHOO.util.Dom.hasClass(this.getEl().getElementsByTagName('a')[0], 'dimmed'))

            { this.hidden = true; }
            this.hiddenStored = null;
            — 526,532 ----
            this.id = this.getEl().id.replace(/module-/i, '');

            this.hidden = false;
            ! if (YAHOO.util.Dom.hasClass(this.getEl().getElementsByTagName('a')[0], 'dimmed') || YAHOO.util.Dom.hasClass(this.getEl().getElementsByTagName('div')[0], 'dimmed_text')) { this.hidden = true; }

            this.hiddenStored = null;

            Show
            hillig Markus Hillig added a comment - For what it's worth it: for Moodle 1.9.2+ I had to adapt Ann's above patch from 07/Nov/07 since now dimmed text gets enclosed between div tags on no longer between span tags. This is what I did. For me it works so far. [root@ecampus html] # diff -c moodle/lib/ajax/section_classes.js.dist moodle/lib/ajax/section_classes.js moodle/lib/ajax/section_classes.js.dist 2008-07-19 02:17:39.000000000 +0200 moodle/lib/ajax/section_classes.js 2008-09-26 16:14:42.000000000 +0200 *************** 526,532 **** this.id = this.getEl().id.replace(/module-/i, ''); this.hidden = false; ! if (YAHOO.util.Dom.hasClass(this.getEl().getElementsByTagName('a') [0] , 'dimmed')) { this.hidden = true; } this.hiddenStored = null; — 526,532 ---- this.id = this.getEl().id.replace(/module-/i, ''); this.hidden = false; ! if (YAHOO.util.Dom.hasClass(this.getEl().getElementsByTagName('a') [0] , 'dimmed') || YAHOO.util.Dom.hasClass(this.getEl().getElementsByTagName('div') [0] , 'dimmed_text')) { this.hidden = true; } this.hiddenStored = null;
            Hide
            nicolasconnault Nicolas Connault added a comment -

            Applied patch to 1.9.3 and HEAD. Works fine.

            Show
            nicolasconnault Nicolas Connault added a comment - Applied patch to 1.9.3 and HEAD. Works fine.
            Hide
            jerome Jérôme Mouneyrac added a comment - - edited

            It works however when I hide the label, the label only becomes grey when I reload the page. Tested on 1.9 with AJAX activated.

            Show
            jerome Jérôme Mouneyrac added a comment - - edited It works however when I hide the label, the label only becomes grey when I reload the page. Tested on 1.9 with AJAX activated.
            Hide
            jerome Jérôme Mouneyrac added a comment -

            I change the priority issue as I confirm that the initial problem has been fixed.

            Show
            jerome Jérôme Mouneyrac added a comment - I change the priority issue as I confirm that the initial problem has been fixed.
            Hide
            jerome Jérôme Mouneyrac added a comment -

            Finally I changed back to Major as the problem still exists on 1.8

            Show
            jerome Jérôme Mouneyrac added a comment - Finally I changed back to Major as the problem still exists on 1.8
            Hide
            nicolasconnault Nicolas Connault added a comment -

            Finally backported this fix to 1.8

            Show
            nicolasconnault Nicolas Connault added a comment - Finally backported this fix to 1.8
            Hide
            skodak Petr Skoda added a comment -

            reopening the label itself is not grayed out after clicking the eye icon

            Show
            skodak Petr Skoda added a comment - reopening the label itself is not grayed out after clicking the eye icon
            Hide
            nicolasconnault Nicolas Connault added a comment -

            Finally fixed!

            Show
            nicolasconnault Nicolas Connault added a comment - Finally fixed!
            Hide
            rwijaya Rossiani Wijaya added a comment -

            Tested and works.

            Resolved.

            Closing.

            Show
            rwijaya Rossiani Wijaya added a comment - Tested and works. Resolved. Closing.

              People

              • Votes:
                5 Vote for this issue
                Watchers:
                4 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved:
                  Fix Release Date:
                  13/May/09