Moodle
  1. Moodle
  2. MDL-20327

Links in framed resource header don't load in _top if javascript is disabled.

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 1.9.5
    • Fix Version/s: 1.9.6, 2.0
    • Component/s: Libraries
    • Labels:
      None
    • Environment:
      UA with no javascript available.
    • Difficulty:
      Easy
    • Affected Branches:
      MOODLE_19_STABLE
    • Fixed Branches:
      MOODLE_19_STABLE, MOODLE_20_STABLE
    • Rank:
      31335

      Description

      When viewing a File Resource using Same window With frame, the breadcrumb links in the header frame load into the header, rather than _top - making the breadcrumb links essentially unusable.

      This is because the target attribute has been replaced with onclick="this.target='_top'", in pursuit of XHTML compliance - at the cost of extra reliance on javascript, arguably in conflict with WCAG 12.1 Graceful transformation of scripts (http://www.w3.org/TR/WCAG10-HTML-TECHS/#scripts

      I'm aware of the Without frame option, but the extra scrollbar which appears with it rather puts me off.

      NB: The target attribute is no longer depreciated in HTML5.

      (As discussed in http://moodle.org/mod/forum/discuss.php?d=132885)

        Issue Links

          Activity

          Hide
          Jérôme Mouneyrac added a comment -

          good catch, fixing it

          Show
          Jérôme Mouneyrac added a comment - good catch, fixing it
          Hide
          Jérôme Mouneyrac added a comment -

          Hi David,
          I looked more into this bug. I'm quite sure this bug is not related directly to resource. In fact it's related to the function navmenu() into weblib which doesn't support no javascript.
          I think you wrote an issue for that: MDL-16158
          Ahah you try to make me fix bug that other didn't want to fix but I caught you I'll still have a look at it.

          Show
          Jérôme Mouneyrac added a comment - Hi David, I looked more into this bug. I'm quite sure this bug is not related directly to resource. In fact it's related to the function navmenu() into weblib which doesn't support no javascript. I think you wrote an issue for that: MDL-16158 Ahah you try to make me fix bug that other didn't want to fix but I caught you I'll still have a look at it.
          Hide
          Jérôme Mouneyrac added a comment -

          after thinking your old bug is just a bit similar, I have a look

          Show
          Jérôme Mouneyrac added a comment - after thinking your old bug is just a bit similar, I have a look
          Hide
          David Balch added a comment -

          Heh, I maintain that weblib patch in my local tree and had forgotten about that other bug!

          Yes, they both relate to target="_top", but this is in the header breadcrumbs, and the other is about autolinks in uploaded files.

          Show
          David Balch added a comment - Heh, I maintain that weblib patch in my local tree and had forgotten about that other bug! Yes, they both relate to target="_top", but this is in the header breadcrumbs, and the other is about autolinks in uploaded files.
          Hide
          Jérôme Mouneyrac added a comment -

          Ok assigned to Sam Navigation Master, the problem is in build_navigation, line 3877:
          $navigation .= "<a onclick=\"this.target='$CFG->framename'\" href=\"{$navlink['link']}\">";
          We need a sort of test on no javascript, something like that
          Thank you

          Show
          Jérôme Mouneyrac added a comment - Ok assigned to Sam Navigation Master, the problem is in build_navigation, line 3877: $navigation .= "<a onclick=\"this.target='$CFG->framename'\" href=\"{$navlink ['link'] }\">"; We need a sort of test on no javascript, something like that Thank you
          Hide
          Sam Hemelryk added a comment -

          Hi Guys,
          Did a quick bit of research on this and it turns out that Petr provided us with a solution back in 2007 when he changed over the target attribute to a JS onclick event.
          If you would prefer to to use the target='..' attribute the simply add the following line into your Moodle site's config.php file somewhere after the call to include setup.php.

          config.php
          
          // This must be added AFTER `require_once($CFG->dirroot/lib/setup.php);`
          // or it will not work
          $CFG->frametarget = " target='$CFG->framename' ";
          
          

          Note: It will not replace the JS onclick event, but simply add a target attribute as well

          Show
          Sam Hemelryk added a comment - Hi Guys, Did a quick bit of research on this and it turns out that Petr provided us with a solution back in 2007 when he changed over the target attribute to a JS onclick event. If you would prefer to to use the target='..' attribute the simply add the following line into your Moodle site's config.php file somewhere after the call to include setup.php. config.php // This must be added AFTER `require_once($CFG->dirroot/lib/setup.php);` // or it will not work $CFG->frametarget = " target='$CFG->framename' " ; Note: It will not replace the JS onclick event, but simply add a target attribute as well
          Hide
          Sam Hemelryk added a comment -

          Commited small patch to ensure CFG->frametarget is used in all navigation code in 1.9 and patched 2.0 to ensure that the same occurs for links using OUTPUT->link (which the navigation does).
          Please update to ensure consistency.
          Thanks all

          Show
          Sam Hemelryk added a comment - Commited small patch to ensure CFG->frametarget is used in all navigation code in 1.9 and patched 2.0 to ensure that the same occurs for links using OUTPUT->link (which the navigation does). Please update to ensure consistency. Thanks all
          Hide
          Martin Dougiamas added a comment -

          OK, so for users you basically ONLY need to just set "framename" in the admin settings GUI.

          Developers need to include $CFG->frametarget in links that could potentially be inside a Moodle frame (eg navigation type things)

          Show
          Martin Dougiamas added a comment - OK, so for users you basically ONLY need to just set "framename" in the admin settings GUI. Developers need to include $CFG->frametarget in links that could potentially be inside a Moodle frame (eg navigation type things)
          Hide
          Martin Dougiamas added a comment -

          I just added patches to 1.9 and 1.8 to FORCE the definition of frametarget to the top frames of resources to completely fix this for the resource module. Sam's hack in config.php a few comments back should never be needed.

          Show
          Martin Dougiamas added a comment - I just added patches to 1.9 and 1.8 to FORCE the definition of frametarget to the top frames of resources to completely fix this for the resource module. Sam's hack in config.php a few comments back should never be needed.
          Hide
          Jérôme Mouneyrac added a comment -

          tested, it works fine, thanks Sam, Martin and David. Closing the issue.

          Show
          Jérôme Mouneyrac added a comment - tested, it works fine, thanks Sam, Martin and David. Closing the issue.
          Hide
          Jérôme Mouneyrac added a comment -

          Note: I just tested on 1.9, this initial issue is fixed when javascript is disabled on 1.9.

          Show
          Jérôme Mouneyrac added a comment - Note: I just tested on 1.9, this initial issue is fixed when javascript is disabled on 1.9.

            People

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

              Dates

              • Created:
                Updated:
                Resolved: