Moodle
  1. Moodle
  2. MDL-30166

Deprecated %fullname% uses still present in core

    Details

    • Testing Instructions:
      Hide
      1. Goto a course>Forum (View.php not discussion.php)
      2. Make sure there are some discussion with posts in the forum.
      3. Make sure you have rss feeds enabled for server, site, and at activity level (for each module that we will be testing)
      4. View the source of the page and make sure you can see an <link rel="alternate" type="application/rss+xml" in the page header
      5. Make sure the title attribute for this is "course : Activityname"
      6. Now go to a discussion page and verify the above holds true for it as well
      7. Goto a glossary view page and verify the above holds true for it as well
      8. view.php for a database with a few entries, and verify the same.
      9. Goto add a DB entry page (edit.php) and verify title as before.
      Show
      Goto a course>Forum (View.php not discussion.php) Make sure there are some discussion with posts in the forum. Make sure you have rss feeds enabled for server, site, and at activity level (for each module that we will be testing) View the source of the page and make sure you can see an <link rel="alternate" type="application/rss+xml" in the page header Make sure the title attribute for this is "course : Activityname" Now go to a discussion page and verify the above holds true for it as well Goto a glossary view page and verify the above holds true for it as well view.php for a database with a few entries, and verify the same. Goto add a DB entry page (edit.php) and verify title as before.
    • Difficulty:
      Easy
    • Affected Branches:
      MOODLE_20_STABLE, MOODLE_21_STABLE, MOODLE_22_STABLE
    • Fixed Branches:
      MOODLE_23_STABLE, MOODLE_24_STABLE
    • Pull 2.4 Branch:
      MDL-30166-m24
    • Pull Master Branch:
      MDL-30166-master
    • Rank:
      26010

      Description

      I was looking the html of one page and saw this:

      <link rel="alternate" type="application/rss+xml" title="Using Moodle: %fullname%" .... />
      

      Looking for %fullname% in code base I got that it is still being used by:

      • lang/en/moodle.php
      • mod/data/edit.php
      • mod/data/view.php
      • mod/forum/discuss.php
      • mod/forum/view.php
      • mod/glossary/view.php

      And the code @ lib/pagelib.php says it's deprecated (so the "template replacement" does not happen anymore), so all uses in the files above should be changed to don't use %fullname% at all.

      1. patch.txt
        4 kB
        Matheus Kautzmann

        Issue Links

          Activity

          Hide
          Matheus Kautzmann added a comment -

          Would you mind posting the html file that you found this?

          Show
          Matheus Kautzmann added a comment - Would you mind posting the html file that you found this?
          Hide
          Matheus Kautzmann added a comment -

          I think I fixed the problem, will be watching this until resolved. Patch is attached.

          Show
          Matheus Kautzmann added a comment - I think I fixed the problem, will be watching this until resolved. Patch is attached.
          Hide
          Ankit Agarwal added a comment -

          Hi Matheus,
          Thanks for providing a patch, however, I have re-written the patch as the supplied patch was conflicting at many places in the current code. Also we should not be using coursname as in the patch, instead we should be using activity name.

          Thanks

          Show
          Ankit Agarwal added a comment - Hi Matheus, Thanks for providing a patch, however, I have re-written the patch as the supplied patch was conflicting at many places in the current code. Also we should not be using coursname as in the patch, instead we should be using activity name. Thanks
          Hide
          Ankit Agarwal added a comment -

          I didn't find any usage of the string pageheaderconfigablock in core, but I have fixed it anyway as well. Also done some minor improvement to remove redundant code.

          Submitting for a review.
          Thanks

          Show
          Ankit Agarwal added a comment - I didn't find any usage of the string pageheaderconfigablock in core, but I have fixed it anyway as well. Also done some minor improvement to remove redundant code. Submitting for a review. Thanks
          Hide
          Rossiani Wijaya added a comment -

          [y] Syntax
          [-] Output
          [y] Whitespace
          [-] Language
          [-] Databases
          [y] Testing
          [-] Security
          [-] Documentation
          [y] Git
          [y] Sanity check

          Patch looks great Ankit.

          Feel free to submit it for integration review.

          Show
          Rossiani Wijaya added a comment - [y] Syntax [-] Output [y] Whitespace [-] Language [-] Databases [y] Testing [-] Security [-] Documentation [y] Git [y] Sanity check Patch looks great Ankit. Feel free to submit it for integration review.
          Hide
          Eloy Lafuente (stronk7) added a comment -

          format_String() ==> amend that upper, plz....otherwise looks perfect.

          Show
          Eloy Lafuente (stronk7) added a comment - format_String() ==> amend that upper, plz....otherwise looks perfect.
          Hide
          Ankit Agarwal added a comment -

          Updated,
          Thanks

          Show
          Ankit Agarwal added a comment - Updated, Thanks
          Hide
          Eloy Lafuente (stronk7) added a comment - - edited

          About the "pageheaderconfigablock" string, it seems that was used initially by blocks, but with MDL-19010 (e03c0c1d49dad7f1d7ccaecee33dfde324f3027f) it became unused since 2.0, but was never deleted.

          So surely it would be interesting to take rid of it. Adding Helen here to know/consider about that.
          (we can add easily one commit to this instead of creating a full new issue, as you prefer).

          Ciao

          Show
          Eloy Lafuente (stronk7) added a comment - - edited About the "pageheaderconfigablock" string, it seems that was used initially by blocks, but with MDL-19010 (e03c0c1d49dad7f1d7ccaecee33dfde324f3027f) it became unused since 2.0, but was never deleted. So surely it would be interesting to take rid of it. Adding Helen here to know/consider about that. (we can add easily one commit to this instead of creating a full new issue, as you prefer). Ciao
          Hide
          Eloy Lafuente (stronk7) added a comment -

          I've created MDL-38451 about the "pageheaderconfigablock" string, FYI.

          Show
          Eloy Lafuente (stronk7) added a comment - I've created MDL-38451 about the "pageheaderconfigablock" string, FYI.
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Integrated (23, 24 & master), thanks!

          Show
          Eloy Lafuente (stronk7) added a comment - Integrated (23, 24 & master), thanks!
          Hide
          Jason Fowler added a comment -

          All good Ankit.

          Show
          Jason Fowler added a comment - All good Ankit.
          Hide
          Eloy Lafuente (stronk7) added a comment -

          This issue has been integrated upstream and is now available both via git and cvs (and in some hours, via mirrors and downloads).

          Thanks!

          PS: Yay, legacy template messages. Yes, you're ok, we don't have CVS anymore!

          Show
          Eloy Lafuente (stronk7) added a comment - This issue has been integrated upstream and is now available both via git and cvs (and in some hours, via mirrors and downloads). Thanks! PS: Yay, legacy template messages. Yes, you're ok, we don't have CVS anymore!

            People

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

              Dates

              • Created:
                Updated:
                Resolved: