Details

    • Type: Sub-task Sub-task
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 2.3.1
    • Fix Version/s: 2.4
    • Component/s: Accessibility
    • Labels:
    • Testing Instructions:
      Hide
      1. Goto course>reports>logs and select all days so you can have multiple pages of result
      2. goto any page other than 1 and make sure the title tag in the page displays the page number
      3. Goto siteadmin>reports>logs and select all days so you can have multiple pages of result
      4. goto any page other than 1 and make sure the title tag in the page displays the page number
      Show
      Goto course>reports>logs and select all days so you can have multiple pages of result goto any page other than 1 and make sure the title tag in the page displays the page number Goto siteadmin>reports>logs and select all days so you can have multiple pages of result goto any page other than 1 and make sure the title tag in the page displays the page number
    • Affected Branches:
      MOODLE_23_STABLE
    • Fixed Branches:
      MOODLE_24_STABLE
    • Pull Master Branch:
      MDL-35225-master
    • Rank:
      43869

      Description

      Multipage items (e.g. Book, Activity Reports, Calendar) should have their page titles change to indicate to the user which page of a resource they are viewing instead of always keeping the same title.

      Greg's comment in MDL-30896 suggest
      For the reports (like the logs) the page title should be something more like
      "Logs: Page 3" or
      "Logs: 1/20/12 12:27PM - 1/20/12 4:12PM"

      instead of every page just saying "Logs"

        Activity

        Hide
        Ankit Agarwal added a comment -

        will upload stable branches once peer-review is done.
        Thanks

        Show
        Ankit Agarwal added a comment - will upload stable branches once peer-review is done. Thanks
        Hide
        Frédéric Massart added a comment - - edited

        Hi Ankit, although your patch looks good, I highly discourage the concatenation of strings, especially when using a colon which is not used in some languages, or require other an extra space before (in French for instance). I know that most of the area in Moodle do not take care of this, but we should definitely think about it. Cheers! (Edit: forgot to mention the unneeded new line on line 121)

        Show
        Frédéric Massart added a comment - - edited Hi Ankit, although your patch looks good, I highly discourage the concatenation of strings, especially when using a colon which is not used in some languages, or require other an extra space before (in French for instance). I know that most of the area in Moodle do not take care of this, but we should definitely think about it. Cheers! (Edit: forgot to mention the unneeded new line on line 121)
        Hide
        Ankit Agarwal added a comment -

        Hi Fred,
        Thanks for your feedback.

        Yes normally we donot concatenate strings in Moodle. But for page titles, I am afraid it needs to be done. Afaik, I confirmed this with Dan, as well earlier, that in case of titles it is alright, but should never be done in content.

        Will update the patch to remove extra line, and push up stable branches.

        Thanks

        Show
        Ankit Agarwal added a comment - Hi Fred, Thanks for your feedback. Yes normally we donot concatenate strings in Moodle. But for page titles, I am afraid it needs to be done. Afaik, I confirmed this with Dan, as well earlier, that in case of titles it is alright, but should never be done in content. Will update the patch to remove extra line, and push up stable branches. Thanks
        Hide
        Eloy Lafuente (stronk7) added a comment -

        The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week.

        TIA and ciao

        Show
        Eloy Lafuente (stronk7) added a comment - The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week. TIA and ciao
        Hide
        Ankit Agarwal added a comment -

        rebased.
        Thanks

        Show
        Ankit Agarwal added a comment - rebased. Thanks
        Hide
        Dan Poltawski added a comment -

        Hmm, well this is a simple issue, but i'm afraid I have some reservations about this.

        OK, so we now have a 'unique page title', but we've also introduced a new string. That is not without its cost:

        1. Our translators struggle to keep up with our new strings: http://download.moodle.org/langpack/2.3/
        2. Its almost certain that once this patch is applied, in most languages we'll be adding another untranslated string to the title
        3. Is adding a [probably] untranslated string really improving accessibility?
        Show
        Dan Poltawski added a comment - Hmm, well this is a simple issue, but i'm afraid I have some reservations about this. OK, so we now have a 'unique page title', but we've also introduced a new string. That is not without its cost: Our translators struggle to keep up with our new strings: http://download.moodle.org/langpack/2.3/ Its almost certain that once this patch is applied, in most languages we'll be adding another untranslated string to the title Is adding a [probably] untranslated string really improving accessibility?
        Hide
        Dan Poltawski added a comment -

        If there is no existing string, then I think it would be better to apply this to master only.

        Show
        Dan Poltawski added a comment - If there is no existing string, then I think it would be better to apply this to master only.
        Hide
        Ankit Agarwal added a comment -

        Hi Dan,
        Yeah I agree with your explanations. The reason, I provided stable branches as well, because we are supposed to treat accessibility improvements as bugs. If you think the new string wont be useful in stables, please feel free to integrate in master only.
        Thanks

        Show
        Ankit Agarwal added a comment - Hi Dan, Yeah I agree with your explanations. The reason, I provided stable branches as well, because we are supposed to treat accessibility improvements as bugs. If you think the new string wont be useful in stables, please feel free to integrate in master only. Thanks
        Hide
        Dan Poltawski added a comment -

        I'm not a massive fan of this string concaination, but it matches existing behaviour, so i've integrated this.

        2.4 only, because IMO its better to have it in exsiting behaviour than likely untranslated strings.

        Show
        Dan Poltawski added a comment - I'm not a massive fan of this string concaination, but it matches existing behaviour, so i've integrated this. 2.4 only, because IMO its better to have it in exsiting behaviour than likely untranslated strings.
        Hide
        Tim Barker added a comment -

        Congrats Ankit, your bug is fixed and has passed the final hurdle, well done

        Show
        Tim Barker added a comment - Congrats Ankit, your bug is fixed and has passed the final hurdle, well done
        Hide
        Dan Poltawski added a comment -

        Congratulations, you've done it!

        Thanks, this change is now in the latest weekly release!

        Join the crowds of people tomorrow from 8am and download this Moodle release from your local apple store!

        Show
        Dan Poltawski added a comment - Congratulations, you've done it! Thanks, this change is now in the latest weekly release! Join the crowds of people tomorrow from 8am and download this Moodle release from your local apple store!

          People

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

            Dates

            • Created:
              Updated:
              Resolved: