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

When viewing a single section, the page title does not update

    Details

    • Testing Instructions:
      Hide
      1. Create a course with a week format
      2. Set the display to one section per page
      3. View the section's page
      4. Ensure the name of the Section/Topic is in the browsers page/tab title.
      • Repeat for other formats that support sections.
      Show
      Create a course with a week format Set the display to one section per page View the section's page Ensure the name of the Section/Topic is in the browsers page/tab title. Repeat for other formats that support sections.
    • Affected Branches:
      MOODLE_23_STABLE
    • Fixed Branches:
      MOODLE_24_STABLE, MOODLE_25_STABLE, MOODLE_26_STABLE, MOODLE_27_STABLE
    • Pull from Repository:
    • Pull Master Branch:
      MDL-37679-master
    • Sprint:
      FRONTEND Sprint 7
    • Story Points (Obsolete):
      5
    • Sprint:
      FRONTEND Sprint 7

      Description

      Issue
      When a user clicks to view an individual section, the page title for the individual section is the course name not the course name plus the section name that the user is viewing. This means that a user does not know they have changed pages.

      Standard Level
      WCAG 2 2.4.2 (A) http://www.w3.org/TR/UNDERSTANDING-WCAG20/navigation-mechanisms-title.html

        Gliffy Diagrams

          Attachments

            Activity

            Hide
            bushido Mark Nielsen added a comment -

            Attaching a possible fix to get creative juices flowing.

            Show
            bushido Mark Nielsen added a comment - Attaching a possible fix to get creative juices flowing.
            Hide
            salvetore Michael de Raadt added a comment -

            I'm shifting this issue into a new Epic issue, where we are collecting together accessibility issues. 218225232

            Show
            salvetore Michael de Raadt added a comment - I'm shifting this issue into a new Epic issue, where we are collecting together accessibility issues. 218225232
            Hide
            phalacee Jason Fowler added a comment -

            Bumping this issues story points up, as it needs to take in to account RTL and string concatenation.

            Show
            phalacee Jason Fowler added a comment - Bumping this issues story points up, as it needs to take in to account RTL and string concatenation.
            Hide
            phalacee Jason Fowler added a comment -

            This has been a master only patch because of the new strings introduced.

            Show
            phalacee Jason Fowler added a comment - This has been a master only patch because of the new strings introduced.
            Hide
            phalacee Jason Fowler added a comment -

            After a discussion in scrum:
            "This should be backported, or at least a patch for older branches should be provided, even if it is a nasty way of doing it, as long as it is no worse than what was there to start with it should be fine"

            I have provided patches for the stable builds. The integrators can have a gladiator style contest to decide whether they go in or not.

            I argue that this is an improvement, not a bug, as there is no real lost of functionality without it, and it only improves accessibility.

            Show
            phalacee Jason Fowler added a comment - After a discussion in scrum: "This should be backported, or at least a patch for older branches should be provided, even if it is a nasty way of doing it, as long as it is no worse than what was there to start with it should be fine" I have provided patches for the stable builds. The integrators can have a gladiator style contest to decide whether they go in or not. I argue that this is an improvement, not a bug, as there is no real lost of functionality without it, and it only improves accessibility.
            Hide
            phalacee Jason Fowler added a comment -

            The stable patches are exactly as Mark wrote them, so I have credited him with the patch in the author field on git.

            Show
            phalacee Jason Fowler added a comment - The stable patches are exactly as Mark wrote them, so I have credited him with the patch in the author field on git.
            Hide
            fred Frédéric Massart added a comment -

            Hi Jason,

            I wasn't part of the SCRUM, but I would say that this should be backported, and perhaps even backported even though there are new strings. If we are not changing existing strings, I think it's fine to backport them. But anyway, you do not need new strings.

            For 2.5 onwards you might want to make use of the course format to get the section name. I am not sure about the correct way to get it, but it seems to me that the course format (which is already instantiated a few lines above) is the perfect way to get the section name without having to call modinfo etc... (see format_base::get_section_name).

            In regard with the string itself, I would not add "Topic: " to it, as it might sound a bit odd with the default Topic format. "Course Biology, Topic: Topic 1", to me it seems good enough to have "Course Biology, Topic 1" or "Course Mathematics, 29 September - 5 October".

            If ever you were not using the course format to get the section name, then you could probably use the modinfo which is returned a few lines below your patch, to prevent double operations.

            I would also ask the tester to try multiple course formats, multiple entry points and making sure it still looks good on a multiple section per page. Not to mention the toggle between editing on and off which changes the appearance too.

            Cheers,
            Fred

            Show
            fred Frédéric Massart added a comment - Hi Jason, I wasn't part of the SCRUM, but I would say that this should be backported, and perhaps even backported even though there are new strings. If we are not changing existing strings, I think it's fine to backport them. But anyway, you do not need new strings. For 2.5 onwards you might want to make use of the course format to get the section name. I am not sure about the correct way to get it, but it seems to me that the course format (which is already instantiated a few lines above) is the perfect way to get the section name without having to call modinfo etc... (see format_base::get_section_name). In regard with the string itself, I would not add "Topic: " to it, as it might sound a bit odd with the default Topic format. "Course Biology, Topic: Topic 1", to me it seems good enough to have "Course Biology, Topic 1" or "Course Mathematics, 29 September - 5 October". If ever you were not using the course format to get the section name, then you could probably use the modinfo which is returned a few lines below your patch, to prevent double operations. I would also ask the tester to try multiple course formats, multiple entry points and making sure it still looks good on a multiple section per page. Not to mention the toggle between editing on and off which changes the appearance too. Cheers, Fred
            Hide
            phalacee Jason Fowler added a comment -

            I've got the course format thing working on Master, will back port that change to 2.6 and 2.5.

            The strings need something to split the Course title and the Topic title, hence the "Topic: ". I considered setting it to Section instead, but the original patch had topic so I left it at that.

            Editing on doesn't change the <title> on a page. It's not about appearance, it's text that is added to the <title> tag only.

            Show
            phalacee Jason Fowler added a comment - I've got the course format thing working on Master, will back port that change to 2.6 and 2.5. The strings need something to split the Course title and the Topic title, hence the "Topic: ". I considered setting it to Section instead, but the original patch had topic so I left it at that. Editing on doesn't change the <title> on a page. It's not about appearance, it's text that is added to the <title> tag only.
            Hide
            phalacee Jason Fowler added a comment -

            I've got it so it gets the topic/section/week string from the course format lang file now.

            Show
            phalacee Jason Fowler added a comment - I've got it so it gets the topic/section/week string from the course format lang file now.
            Hide
            phalacee Jason Fowler added a comment -

            All those changes have been made Fred, pushing for integration

            Show
            phalacee Jason Fowler added a comment - All those changes have been made Fred, pushing for integration
            Hide
            fred Frédéric Massart added a comment - - edited

            Thanks Jason. I know I'm not a native speaker, but "Course: Hamburgers, Topic: Topic 1" sound funny to me. Anyway, thanks! Also, course_get_format() is called twice in this document, perhaps we could prevent that.

            Show
            fred Frédéric Massart added a comment - - edited Thanks Jason. I know I'm not a native speaker, but "Course: Hamburgers, Topic: Topic 1" sound funny to me. Anyway, thanks! Also, course_get_format() is called twice in this document, perhaps we could prevent that.
            Hide
            phalacee Jason Fowler added a comment -

            Ah, thanks for that Fred, all fixed.

            Show
            phalacee Jason Fowler added a comment - Ah, thanks for that Fred, all fixed.
            Hide
            marina Marina Glancy added a comment -

            Hi Jason, thanks for fixing it.

            First, function format_base::get_section_name() does not have a second argument, but you do not need to use this function anyway.

            It is better to use global function get_section_name($course, $section), do not worry about calling get_course_format() multiple times, it has caching inside. It can accept sectionid as well as section object in all versions including 2.4. So there is no need to call get_fast_modinfo(). Diff for 2.4-2.6 must be absolutely the same.

            Can you please make sure that you call if (course_format_uses_sections()) before calling get_string('sectionname', "format_$course->format"), otherwise you might get debugging messages because course formats that do not support sections do not have to define this string.

            Make sure to include in testing instructions formats that do not support sections as well, such as single activity and social. By the way, single activity format may have "orphaned" activities and they would be displayed on the page course/view.php?id=2&section=1

            Show
            marina Marina Glancy added a comment - Hi Jason, thanks for fixing it. First, function format_base::get_section_name() does not have a second argument, but you do not need to use this function anyway. It is better to use global function get_section_name($course, $section), do not worry about calling get_course_format() multiple times, it has caching inside. It can accept sectionid as well as section object in all versions including 2.4. So there is no need to call get_fast_modinfo(). Diff for 2.4-2.6 must be absolutely the same. Can you please make sure that you call if (course_format_uses_sections()) before calling get_string('sectionname', "format_$course->format"), otherwise you might get debugging messages because course formats that do not support sections do not have to define this string. Make sure to include in testing instructions formats that do not support sections as well, such as single activity and social. By the way, single activity format may have "orphaned" activities and they would be displayed on the page course/view.php?id=2&section=1
            Hide
            phalacee Jason Fowler added a comment - - edited

            Can you please make sure that you call if (course_format_uses_sections()) before calling get_string('sectionname', "format_$course->format"), otherwise you might get debugging messages because course formats that do not support sections do not have to define this string.

            Formats that do not support sections will never link to a section page. Thus this will never be triggered. It's covered off by the

            if ($section and $section > 0) 

            Edit: just read the bit about the orphaned activities ... looking at that now then...

            Show
            phalacee Jason Fowler added a comment - - edited Can you please make sure that you call if (course_format_uses_sections()) before calling get_string('sectionname', "format_$course->format"), otherwise you might get debugging messages because course formats that do not support sections do not have to define this string. Formats that do not support sections will never link to a section page. Thus this will never be triggered. It's covered off by the if ($section and $section > 0) Edit: just read the bit about the orphaned activities ... looking at that now then...
            Hide
            phalacee Jason Fowler added a comment -

            okay, all those changes have been made now Marina.

            Show
            phalacee Jason Fowler added a comment - okay, all those changes have been made now Marina.
            Hide
            marina Marina Glancy added a comment -

            Thanks Jason, integrated in 2.4, 2.5, 2.6 and master

            Show
            marina Marina Glancy added a comment - Thanks Jason, integrated in 2.4, 2.5, 2.6 and master
            Hide
            abgreeve Adrian Greeve added a comment -

            Tested on the 2.4, 2.5, 2.6 and master integration branches.
            No problems found. Test passed.

            Show
            abgreeve Adrian Greeve added a comment - Tested on the 2.4, 2.5, 2.6 and master integration branches. No problems found. Test passed.
            Hide
            damyon Damyon Wiese added a comment -

            Twas the week before Christmas,
            And all though HQ
            Devs were scrambling to finish peer review.
            They sent all their issues,
            and rushed out the door -
            "To the beach!" someone heard them roar!

            This issue has been released upstream. Thanks!

            Show
            damyon Damyon Wiese added a comment - Twas the week before Christmas, And all though HQ Devs were scrambling to finish peer review. They sent all their issues, and rushed out the door - "To the beach!" someone heard them roar! This issue has been released upstream. Thanks!

              People

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

                Dates

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