Details

    • Testing Instructions:
      Hide
      1. Select Clean theme
      2. In any browser use 'Print Preview' of the front page and observe that the link URL's are appended to the text.
      3. Apply the patch.
      4. In any browser use 'Print Preview' of the front page and observe that the link URL's are no longer appended to the text.
      Show
      Select Clean theme In any browser use 'Print Preview' of the front page and observe that the link URL's are appended to the text. Apply the patch. In any browser use 'Print Preview' of the front page and observe that the link URL's are no longer appended to the text.
    • Workaround:
      Hide

      Use the following CSS in the 'Clean' theme's custom CSS setting:

      @media print {
        a[href]:after {
          content: "";
        }
      }
      
      Show
      Use the following CSS in the 'Clean' theme's custom CSS setting: @media print { a[href]:after { content: ""; } }
    • Affected Branches:
      MOODLE_25_STABLE
    • Fixed Branches:
      MOODLE_25_STABLE, MOODLE_26_STABLE
    • Pull from Repository:
    • Pull 2.5 Branch:
      wip-MDL-40321_M25
    • Pull Master Branch:
      wip-MDL-40321_master
    • Rank:
      51098

      Description

      The Bootstrap CSS pulls in some print styles that append the destination URL to links in the printout. This is helpful for printed content, but needs some kind of exception to stop it doing it for Moodle internal links.

      I added an exception for our institution by using this rule:

      a[href^="http://startofyourmoodle.url.com"]:after

      { content:""}

      But that doesn't generalise to any Moodle. Perhaps completely negating the print rule (which is in theme/bootstrapbase/less/reset.less) might be a good idea. The rule could then be re-instated but targetting only Moodle content in some way?

      1. mdl_40321_clean_1.png
        104 kB
      2. mdl_40321_clean_2.png
        85 kB
      3. mdl_40321_standard_1.png
        117 kB
      4. mdl_40321_standard_2.png
        126 kB
      5. mdl_40321_standard_3.png
        97 kB

        Issue Links

          Activity

          Hide
          Gareth J Barnard added a comment -

          I think the links might be a good idea, just perhaps with a tad smaller font.

          Show
          Gareth J Barnard added a comment - I think the links might be a good idea, just perhaps with a tad smaller font.
          Hide
          Mary Evans added a comment -

          This needs sorting out quick!

          Show
          Mary Evans added a comment - This needs sorting out quick!
          Hide
          Gareth J Barnard added a comment -

          Just had the thought that David Scotson's solution could be implemented in 'Clean' with CSS pre-processing - just like the original font serving mechanism:

            a[href^="[[theme:wwwroot]]"]:after {
              content: "";
            }
          

          in custom.css, then in lib.php:

          function theme_clean_set_wwwroot($css) {
              global $CFG;
              $tag = '[[theme:wwwroot]]';
              $css = str_replace($tag, $CFG->wwwroot, $css);
          
              return $css;
          }
          

          You get the idea...

          Not sure about 'https' though?

          Gareth

          Show
          Gareth J Barnard added a comment - Just had the thought that David Scotson 's solution could be implemented in 'Clean' with CSS pre-processing - just like the original font serving mechanism: a[href^= "[[theme:wwwroot]]" ]:after { content: ""; } in custom.css, then in lib.php: function theme_clean_set_wwwroot($css) { global $CFG; $tag = '[[theme:wwwroot]]'; $css = str_replace($tag, $CFG->wwwroot, $css); return $css; } You get the idea... Not sure about 'https' though? Gareth
          Hide
          Gareth J Barnard added a comment -

          But, this might be overkill, as you could have a 'moodle/reset.less' with:

          @media print {
            a[href]:after {
              content: "";
            }
          }
          

          which would be included after 'bootstrap/reset.less', but it would mean that no links would be printed.

          Show
          Gareth J Barnard added a comment - But, this might be overkill, as you could have a 'moodle/reset.less' with: @media print { a[href]:after { content: ""; } } which would be included after 'bootstrap/reset.less', but it would mean that no links would be printed.
          Hide
          Mary Evans added a comment - - edited

          Just logging Greg's comments from MDL-40602

          https://tracker.moodle.org/browse/MDL-40602?focusedCommentId=240401&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-240401

          And his suggested fix...

          Change code in theme/bootstrapbase/less/bootstrap/reset.less from:
           

            
            a[href]:after {
                content: " (" attr(href) ")";
            }
          

          to this:
           

            a[href]:after {
                content: none;
            }
          
          
          Show
          Mary Evans added a comment - - edited Just logging Greg's comments from MDL-40602 https://tracker.moodle.org/browse/MDL-40602?focusedCommentId=240401&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-240401 And his suggested fix... Change code in theme/bootstrapbase/less/bootstrap/reset.less from:   a[href]:after { content: " (" attr(href) ")" ; } to this:   a[href]:after { content: none; }
          Hide
          Mary Evans added a comment - - edited

          This makes sense as the above code is inside an @media print at line 167

          https://github.com/lazydaisy/moodle/blob/master/theme/bootstrapbase/less/bootstrap/reset.less#L167

          Show
          Mary Evans added a comment - - edited This makes sense as the above code is inside an @media print at line 167 https://github.com/lazydaisy/moodle/blob/master/theme/bootstrapbase/less/bootstrap/reset.less#L167
          Hide
          Gareth J Barnard added a comment -

          But I thought for maintainability reasons every 'less' file in the 'bootstrap' folder was untouchable?

          Show
          Gareth J Barnard added a comment - But I thought for maintainability reasons every 'less' file in the 'bootstrap' folder was untouchable?
          Hide
          Gareth J Barnard added a comment -

          Submitting for Peer Review of fix in Moodle 2.5 branch, if acceptable I'll create a 'master' version before requesting submission for integration.

          Show
          Gareth J Barnard added a comment - Submitting for Peer Review of fix in Moodle 2.5 branch, if acceptable I'll create a 'master' version before requesting submission for integration.
          Hide
          Mary Evans added a comment -

          OK...starting Peer Review

          Show
          Mary Evans added a comment - OK...starting Peer Review
          Hide
          Mary Evans added a comment -

          +1 for the code, it looks good and works OK, so submitting for Integration Review.

          Please do add a master branch that would be good.

          Thanks
          Mary

          Show
          Mary Evans added a comment - +1 for the code, it looks good and works OK, so submitting for Integration Review. Please do add a master branch that would be good. Thanks Mary
          Hide
          Gareth J Barnard added a comment - - edited

          Will do tomorrow. I've updated the compare as it was looking at my MOODLE_25_STABLE branch and not the Moodle repository one. And thanks for peer reviewing Mary Evans

          Show
          Gareth J Barnard added a comment - - edited Will do tomorrow. I've updated the compare as it was looking at my MOODLE_25_STABLE branch and not the Moodle repository one. And thanks for peer reviewing Mary Evans
          Hide
          Mary Evans added a comment - - edited

          @Gareth, The compare should be with your repository's updated MOODLE_25_STABLE branch, this should be the same branch that you based your patch on. If this is not the case then I will have to reopen this until your branches are both uptodate and based on their respective branches.

          Show
          Mary Evans added a comment - - edited @Gareth, The compare should be with your repository's updated MOODLE_25_STABLE branch, this should be the same branch that you based your patch on. If this is not the case then I will have to reopen this until your branches are both uptodate and based on their respective branches.
          Hide
          Mary Evans added a comment -
          Show
          Mary Evans added a comment - You should write it like this... https://github.com/gjb2048/moodle/compare/MOODLE_25_STABLE...wip-MDL-40321_M25
          Hide
          Gareth J Barnard added a comment -

          They are up to date

          Show
          Gareth J Barnard added a comment - They are up to date
          Hide
          Mary Evans added a comment -

          I can see that I was just wondering why you changed the link

          Show
          Mary Evans added a comment - I can see that I was just wondering why you changed the link
          Hide
          Gareth J Barnard added a comment -

          Because locally I had based the wip-MDL-40312_M25 branch upon a latest MOODLE_25_BRANCH but had only pushed the former remotely and not the latter so the compare was wrong. So I thought the compare URL was incorrect. I've since pushed MOODLE_25_STABLE and hence the corrected URL compare is correct.

          Show
          Gareth J Barnard added a comment - Because locally I had based the wip- MDL-40312 _M25 branch upon a latest MOODLE_25_BRANCH but had only pushed the former remotely and not the latter so the compare was wrong. So I thought the compare URL was incorrect. I've since pushed MOODLE_25_STABLE and hence the corrected URL compare is correct.
          Hide
          Damyon Wiese added a comment -

          Thanks Gareth,

          This has been integrated to 25 and master now.

          Note - I added a commit to keep the less files tidy - we recently added a bootstrapoverrides.less file which has the same purpose as your reset.less file - so I moved your rules into that one and got rid of reset.less.

          The bootstrapoverrides.less file was not backported to 25 - but I added it on that branch anyway to keep the branches sort of in sync.

          Show
          Damyon Wiese added a comment - Thanks Gareth, This has been integrated to 25 and master now. Note - I added a commit to keep the less files tidy - we recently added a bootstrapoverrides.less file which has the same purpose as your reset.less file - so I moved your rules into that one and got rid of reset.less. The bootstrapoverrides.less file was not backported to 25 - but I added it on that branch anyway to keep the branches sort of in sync.
          Hide
          Gareth J Barnard added a comment -

          Dear Damyon Wiese,

          No problem and thanks .

          I named it 'reset.less' to be consistent with the Bootstrap names.

          Cheers,

          Gareth

          Show
          Gareth J Barnard added a comment - Dear Damyon Wiese , No problem and thanks . I named it 'reset.less' to be consistent with the Bootstrap names. Cheers, Gareth
          Hide
          Jérôme Mouneyrac added a comment -

          On the print preview (linux/chrome), with the Standard theme, we can see the texts as blue link. On the Clean theme we don't know anymore that these texts are links. I prefer the look with links but it removes the information of the text been a link. The second issue is that the themes are not consistent too. Personally I have no problem with that, but just to be sure I fail it to know the opinion of integrator. You can pass it straight away if it's not a problem.

          Show
          Jérôme Mouneyrac added a comment - On the print preview (linux/chrome), with the Standard theme, we can see the texts as blue link. On the Clean theme we don't know anymore that these texts are links. I prefer the look with links but it removes the information of the text been a link. The second issue is that the themes are not consistent too. Personally I have no problem with that, but just to be sure I fail it to know the opinion of integrator. You can pass it straight away if it's not a problem.
          Hide
          Damyon Wiese added a comment -

          Good point Jerome - I think colouring the links for the print style sheet is a good idea.

          Show
          Damyon Wiese added a comment - Good point Jerome - I think colouring the links for the print style sheet is a good idea.
          Hide
          Mary Evans added a comment -

          Damyon/Jerome,

          Whilst I agree about making a link distinguishable when printing, I disagree about colour, bold maybe, but colour no. I only say this as I always print in black and white to save on colored ink.

          If you notice also that some themes do not specify a:link/a:visited these they leave to the browser default, which in most cases is blue/red mine is bright pink/yellow so they stand out while theming, as I prefer to style them to match the theme.

          So my -1 for colour but am happier if links are bold.

          Thank you.
          Mary

          Show
          Mary Evans added a comment - Damyon/Jerome, Whilst I agree about making a link distinguishable when printing, I disagree about colour, bold maybe, but colour no. I only say this as I always print in black and white to save on colored ink. If you notice also that some themes do not specify a:link/a:visited these they leave to the browser default, which in most cases is blue/red mine is bright pink/yellow so they stand out while theming, as I prefer to style them to match the theme. So my -1 for colour but am happier if links are bold . Thank you. Mary
          Hide
          Gareth J Barnard added a comment -

          Ok, regardless of what happens at the moment, given what has been said I consider that the links should be coloured as you can always choose to print in black and white, bold uses more ink if you do and internal links should not be printed as they are pointless and the real reason the issue was raised in the first place. So later today I'll consider how the CSS pre-processing mechanism above could be employed. Tips on the https issue appreciated.

          Show
          Gareth J Barnard added a comment - Ok, regardless of what happens at the moment, given what has been said I consider that the links should be coloured as you can always choose to print in black and white, bold uses more ink if you do and internal links should not be printed as they are pointless and the real reason the issue was raised in the first place. So later today I'll consider how the CSS pre-processing mechanism above could be employed. Tips on the https issue appreciated.
          Hide
          Gareth J Barnard added a comment -

          And what I've said above could apply to all themes.

          Show
          Gareth J Barnard added a comment - And what I've said above could apply to all themes.
          Hide
          Gareth J Barnard added a comment - - edited

          Ok, I've done some investigation and attached some images showing links in the Standard and Clean themes without the patch in Moodle 2.5. It appears that by default the links in the Standard theme are not printed as you say Jerome - I'm using Chrome Version '29.0.1547.57 m' on windows - looking at the screen shots explictly printed links are there in both themes (this patch removes the horrible extra bracketed ones), it's just in the Clean theme they are not blue (see the mdl_40321_standard_3.png and mdl_40321_clean_1.png screen shots). Therefore I believe you are referring to a different styling issue and not what this patch addresses. Therefore the fix as stands is consistent with the current base based themes and therefore anything else would be an improvement. Therefore I believe that the existing patch should stand. If not then I consider that internal links should not be printed as shown by the 'Clean' screen shots.

          Show
          Gareth J Barnard added a comment - - edited Ok, I've done some investigation and attached some images showing links in the Standard and Clean themes without the patch in Moodle 2.5. It appears that by default the links in the Standard theme are not printed as you say Jerome - I'm using Chrome Version '29.0.1547.57 m' on windows - looking at the screen shots explictly printed links are there in both themes (this patch removes the horrible extra bracketed ones), it's just in the Clean theme they are not blue (see the mdl_40321_standard_3.png and mdl_40321_clean_1.png screen shots). Therefore I believe you are referring to a different styling issue and not what this patch addresses. Therefore the fix as stands is consistent with the current base based themes and therefore anything else would be an improvement. Therefore I believe that the existing patch should stand. If not then I consider that internal links should not be printed as shown by the 'Clean' screen shots.
          Hide
          Damyon Wiese added a comment -

          I've added a new issue for the missing link text decoration in bootstrap.

          Jerome - can you pass this if you are satisfied (back to testing)?

          Show
          Damyon Wiese added a comment - I've added a new issue for the missing link text decoration in bootstrap. Jerome - can you pass this if you are satisfied (back to testing)?
          Hide
          Jérôme Mouneyrac added a comment -

          As Damyon created an issue for the missing blue coloration, passing Thanks guy.

          Show
          Jérôme Mouneyrac added a comment - As Damyon created an issue for the missing blue coloration, passing Thanks guy.
          Hide
          Damyon Wiese added a comment -

          There was a young man named McGee
          Who thought squashing bugs was easy
          He tried it one day
          And to his dismay
          The bug guts made his keyboard all greasy

          Thanks!

          This has issue has been fixed and released in Moodle.

          Show
          Damyon Wiese added a comment - There was a young man named McGee Who thought squashing bugs was easy He tried it one day And to his dismay The bug guts made his keyboard all greasy Thanks! This has issue has been fixed and released in Moodle.

            People

            • Votes:
              7 Vote for this issue
              Watchers:
              8 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved: