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

Unhiding a section does not removed the dimmed class to resources

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Minor
    • Resolution: Fixed
    • Affects Version/s: 2.3.3, 2.4
    • Fix Version/s: 2.3.4
    • Component/s: Course
    • Labels:
    • Testing Instructions:
      Hide

      Run MDLQA-4944 and make sure it pass.

      1. Enable the conditional access (enableavailability)
      2. Have a course section with:
        • A visible resource
        • A hidden resource
        • A resource hidden (but visible, only dimmed) by conditional access
      3. Hide the section
      4. Reload the page (this is important!)
      5. Unhide the section
      6. Make sure previously hidden activity is still dimmed and active activity is active.
      Show
      Run MDLQA-4944 and make sure it pass. Enable the conditional access (enableavailability) Have a course section with: A visible resource A hidden resource A resource hidden (but visible, only dimmed) by conditional access Hide the section Reload the page (this is important!) Unhide the section Make sure previously hidden activity is still dimmed and active activity is active.
    • Affected Branches:
      MOODLE_23_STABLE, MOODLE_24_STABLE
    • Fixed Branches:
      MOODLE_23_STABLE
    • Pull Master Branch:
      wip-mdl-36707

      Description

      This is a JavaScript only error, discovered while following MDLQA-4944.

      1. Enable the conditional access (enableavailability)
      2. Have a section with:
        • A visible resource
        • A hidden resource
        • A resource hidden (but visible, only dimmed) by conditional access
      3. Hide the section
      4. Reload the page (this is important!)
      5. Unhide the section

      Expected

      • The previously hidden resources are still dimmed

      Actual

      • The visible resource is dimmed

      I think this is regression caused by MDL-36131 (57bda785403e32864ad90aa63df8a272bfd188ad). The problem appears to be that the 'conditionalhidden' class is always set regardless of whether the resource has a conditional access which hides it. Then the JS will not toggle the status because of that reason. Also, as minor note, I think we should not make use of 'accesshide' for elements which should not be displayed but use 'hide' instead. What would happen there is that the screenreaders would read the information, but that information is unavailable to non-impaired users.

      The exact same result occurs on resources hide/show themselves, try to hide a resource, refresh the page and unhide it to reproduce.

        Gliffy Diagrams

          Attachments

            Issue Links

              Activity

              Hide
              fred Frédéric Massart added a comment -

              Very quick patch.

               
              diff --git a/course/lib.php b/course/lib.php
              index d685223..db37aa4 100644
              --- a/course/lib.php
              +++ b/course/lib.php
              @@ -1427,15 +1427,17 @@ function print_section($course, $section, $mods, $modnamesused, $absolute=false,
                           $modcontext = context_module::instance($mod->id);
                           $canviewhidden = has_capability('moodle/course:viewhiddenactivities', $modcontext);
                           $accessiblebutdim = false;
              +            $conditionalhidden = false;
                           if ($canviewhidden) {
              diff --git a/course/lib.php b/course/lib.php
              index d685223..db37aa4 100644
              --- a/course/lib.php
              +++ b/course/lib.php
              @@ -1427,15 +1427,17 @@ function print_section($course, $section, $mods, $modnamesused, $absolute=false,
                           $modcontext = context_module::instance($mod->id);
                           $canviewhidden = has_capability('moodle/course:viewhiddenactivities', $modcontext);
                           $accessiblebutdim = false;
              +            $conditionalhidden = false;
                           if ($canviewhidden) {
                               $accessiblebutdim = !$mod->visible;
                               if (!empty($CFG->enableavailability)) {
              -                    $accessiblebutdim = $accessiblebutdim ||
              +                    $conditionalhidden =
                                       $mod->availablefrom > time() ||
                                       ($mod->availableuntil && $mod->availableuntil < time()) ||
                                       count($mod->conditionsgrade) > 0 ||
                                       count($mod->conditionscompletion) > 0;
                               }
              +                $accessiblebutdim = $conditionalhidden || $accessiblebutdim;
                           }
               
                           $liclasses = array();
              @@ -1491,8 +1493,12 @@ function print_section($course, $section, $mods, $modnamesused, $absolute=false,
                               $linkclasses = '';
                               $textclasses = '';
                               if ($accessiblebutdim) {
              -                    $linkclasses .= ' dimmed conditionalhidden';
              -                    $textclasses .= ' dimmed_text conditionalhidden';
              +                    $linkclasses .= ' dimmed';
              +                    $textclasses .= ' dimmed_text';
              +                    if ($conditionalhidden) {
              +                        $linkclasses .= ' conditionalhidden';
              +                        $textclasses .= ' conditionalhidden';
              +                    }
                                   $accesstext = '<span class="accesshide">'.
                                       get_string('hiddenfromstudents').': </span>';
                               } else {

              Show
              fred Frédéric Massart added a comment - Very quick patch.   diff --git a/course/lib.php b/course/lib.php index d685223..db37aa4 100644 --- a/course/lib.php +++ b/course/lib.php @@ -1427,15 +1427,17 @@ function print_section($course, $section, $mods, $modnamesused, $absolute=false, $modcontext = context_module::instance($mod->id); $canviewhidden = has_capability('moodle/course:viewhiddenactivities', $modcontext); $accessiblebutdim = false; + $conditionalhidden = false; if ($canviewhidden) { diff --git a/course/lib.php b/course/lib.php index d685223..db37aa4 100644 --- a/course/lib.php +++ b/course/lib.php @@ -1427,15 +1427,17 @@ function print_section($course, $section, $mods, $modnamesused, $absolute=false, $modcontext = context_module::instance($mod->id); $canviewhidden = has_capability('moodle/course:viewhiddenactivities', $modcontext); $accessiblebutdim = false; + $conditionalhidden = false; if ($canviewhidden) { $accessiblebutdim = !$mod->visible; if (!empty($CFG->enableavailability)) { - $accessiblebutdim = $accessiblebutdim || + $conditionalhidden = $mod->availablefrom > time() || ($mod->availableuntil && $mod->availableuntil < time()) || count($mod->conditionsgrade) > 0 || count($mod->conditionscompletion) > 0; } + $accessiblebutdim = $conditionalhidden || $accessiblebutdim; } $liclasses = array(); @@ -1491,8 +1493,12 @@ function print_section($course, $section, $mods, $modnamesused, $absolute=false, $linkclasses = ''; $textclasses = ''; if ($accessiblebutdim) { - $linkclasses .= ' dimmed conditionalhidden'; - $textclasses .= ' dimmed_text conditionalhidden'; + $linkclasses .= ' dimmed'; + $textclasses .= ' dimmed_text'; + if ($conditionalhidden) { + $linkclasses .= ' conditionalhidden'; + $textclasses .= ' conditionalhidden'; + } $accesstext = '<span class="accesshide">'. get_string('hiddenfromstudents').': </span>'; } else {
              Hide
              rajeshtaneja Rajesh Taneja added a comment -

              Thanks Fred,

              Patch make sense and it do make sense to handle conditinalhidden and dimmed class separately.
              Putting patch for peer-review.

              Show
              rajeshtaneja Rajesh Taneja added a comment - Thanks Fred, Patch make sense and it do make sense to handle conditinalhidden and dimmed class separately. Putting patch for peer-review.
              Hide
              fred Frédéric Massart added a comment -

              Thanks Raj. Pushing for integration.

              Show
              fred Frédéric Massart added a comment - Thanks Raj. Pushing for integration.
              Hide
              nebgor Aparup Banerjee added a comment -

              Thanks, thats integrated.

              Show
              nebgor Aparup Banerjee added a comment - Thanks, thats integrated.
              Hide
              skodak Petr Skoda added a comment -

              works fine in both branches, thanks

              Show
              skodak Petr Skoda added a comment - works fine in both branches, thanks
              Hide
              poltawski Dan Poltawski added a comment -

              Congratulations! Another bug solved.. only another 7330 to go, thanks for contributing to contributing to 0.8% of all bugs being fixed this week!

              ciao
              Dan

              Show
              poltawski Dan Poltawski added a comment - Congratulations! Another bug solved.. only another 7330 to go, thanks for contributing to contributing to 0.8% of all bugs being fixed this week! ciao Dan

                People

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

                  Dates

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