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

          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

                Agile