Moodle
  1. Moodle
  2. MDL-37574

Remove default title from navigation items

    Details

    • Testing Instructions:
      Hide
      • Open a course page
      • Inspect a navigation item
        • Confirm that anchor does not have a title attribute
      • Edit /course/renderer.php
        • find function add_modchoosertoggle()
        • after the modchoosertoggle has been defined add:
          $modchoosertoggle->title = 'MDL-37574'
          
      • refresh the course page
      • turn on editing mode
      • inspect the 'Activity chooser on/off' toggle in the Settings block
        • *confirm that the anchor has a title of 'MDL-37574'
      Show
      Open a course page Inspect a navigation item Confirm that anchor does not have a title attribute Edit /course/renderer.php find function add_modchoosertoggle() after the modchoosertoggle has been defined add: $modchoosertoggle->title = 'MDL-37574' refresh the course page turn on editing mode inspect the 'Activity chooser on/off' toggle in the Settings block *confirm that the anchor has a title of ' MDL-37574 '
    • Affected Branches:
      MOODLE_24_STABLE
    • Fixed Branches:
      MOODLE_23_STABLE, MOODLE_24_STABLE
    • Pull 2.4 Branch:
      MDL-37574-m24
    • Pull Master Branch:
    • Rank:
      47228

      Description

      Clicking on activity chooser change text from "Activity chooser on" to "Activity chooser off" but Title and alt-text remains "Activity chooser on"

      Steps to reproduce:

      1. Login as admin
      2. Turn enableajax on (Home ► Site administration ► Appearance ► AJAX and Javascript)
      3. Select a course and turn editing on
      4. Click on Activity chooser, you will see text changing but title and alt-text remain same.

        Issue Links

          Activity

          Hide
          Andrew Nicols added a comment -

          Hmm, indeed you are correct - the title attribute never changes. This is technically a bug in the JavaScript

          However, I've just been reading an interesting article from the (UK-based) Royal National Institute for the Blind on the subject of using the TITLE attribute on links: http://www.rnib.org.uk/professionals/webaccessibility/wacblog/Lists/Posts/Post.aspx?id=38

          I wonder whether we should actually remove the default TITLE attribute from all navigation content.

          I'm not very sure how this sits on the accessibility front, so I'd be interested to hear what someone who knows more thinks.

          Andrew

          Show
          Andrew Nicols added a comment - Hmm, indeed you are correct - the title attribute never changes. This is technically a bug in the JavaScript However, I've just been reading an interesting article from the (UK-based) Royal National Institute for the Blind on the subject of using the TITLE attribute on links: http://www.rnib.org.uk/professionals/webaccessibility/wacblog/Lists/Posts/Post.aspx?id=38 I wonder whether we should actually remove the default TITLE attribute from all navigation content. I'm not very sure how this sits on the accessibility front, so I'd be interested to hear what someone who knows more thinks. Andrew
          Hide
          Rajesh Taneja added a comment -

          Hello Andrew,

          AFAIK, when both title and alt are same (which they are in this case), title should not be there.
          +1 for removing title.

          Show
          Rajesh Taneja added a comment - Hello Andrew, AFAIK, when both title and alt are same (which they are in this case), title should not be there. +1 for removing title.
          Hide
          Andrew Nicols added a comment -

          Chetz,

          I've added you as a watcher on this issue after your talk last week. I thought you may be interested. Am I correct in thinking that the correct thing to do would be to remove the 'title' attribute from the anchor since it identically matches the content of the anchor, or should we be doing something else instead?

          Andrew

          Show
          Andrew Nicols added a comment - Chetz, I've added you as a watcher on this issue after your talk last week. I thought you may be interested. Am I correct in thinking that the correct thing to do would be to remove the 'title' attribute from the anchor since it identically matches the content of the anchor, or should we be doing something else instead? Andrew
          Hide
          Andrew Nicols added a comment -

          I'll provide stable branches following peer review.

          Show
          Andrew Nicols added a comment - I'll provide stable branches following peer review.
          Hide
          Chetz Colwell added a comment -

          I'm not familiar with the activity chooser and haven't tested this, but in principle I don't think there is a need for a title attribute if its text matches the visible text.

          (Title is not really an accessibility feature as I think it only benefits sighted users who see it when they hover the mouse, and I often think that if the title is needed to explain the purpose of a link then maybe the text of the link should be reconsidered!)

          This WCAG2.0 technique article may be useful: http://www.w3.org/TR/WCAG20-TECHS/H33.html.

          Show
          Chetz Colwell added a comment - I'm not familiar with the activity chooser and haven't tested this, but in principle I don't think there is a need for a title attribute if its text matches the visible text. (Title is not really an accessibility feature as I think it only benefits sighted users who see it when they hover the mouse, and I often think that if the title is needed to explain the purpose of a link then maybe the text of the link should be reconsidered!) This WCAG2.0 technique article may be useful: http://www.w3.org/TR/WCAG20-TECHS/H33.html .
          Hide
          Frédéric Massart added a comment -

          Thanks Andrew, removing the title makes sense to me too. Before you push for integration could you add the component to the commit message? Would you consider backporting this issue?

          Cheers,
          Fred

          Show
          Frédéric Massart added a comment - Thanks Andrew, removing the title makes sense to me too. Before you push for integration could you add the component to the commit message? Would you consider backporting this issue? Cheers, Fred
          Hide
          Andrew Nicols added a comment -

          Thanks all,

          Submitting for integration.

          Show
          Andrew Nicols added a comment - Thanks all, Submitting for integration.
          Hide
          Dan Poltawski added a comment -

          Hi Andrew,

          I've integrated this now to 24, 23 and master.

          I see that you didn't mention the component as Fred requested. I think that was good advice from Fred and could've improved the commit message a little (as we can have navigation items in other places but navigaitonlib). Whilst components are not compulsory, where they improve the understandability they are useful.

          Anyway, that wasn't a big enough issue to delay this any longer, just thought I'd let you know.

          Show
          Dan Poltawski added a comment - Hi Andrew, I've integrated this now to 24, 23 and master. I see that you didn't mention the component as Fred requested. I think that was good advice from Fred and could've improved the commit message a little (as we can have navigation items in other places but navigaitonlib). Whilst components are not compulsory, where they improve the understandability they are useful. Anyway, that wasn't a big enough issue to delay this any longer, just thought I'd let you know.
          Show
          Dan Poltawski added a comment - This is breaking phpunit on all branches: http://integration.moodle.org/job/07.%20Run%20phpunit%20UnitTests%20(23_STABLE)/lastCompletedBuild/testReport/(root)/navigation_node_testcase/test___construct/ http://integration.moodle.org/job/07.%20Run%20phpunit%20UnitTests%20(24_STABLE)/lastCompletedBuild/testReport/(root)/navigation_node_testcase/test___construct/ http://integration.moodle.org/job/07.%20Run%20phpunit%20UnitTests%20(master)/lastCompletedBuild/testReport/(root)/navigation_node_testcase/test___construct/
          Hide
          Dan Poltawski added a comment -

          Bah, no fixes arrived for this.

          Show
          Dan Poltawski added a comment - Bah, no fixes arrived for this.
          Hide
          Dan Poltawski added a comment -

          I've pushed a fix for this (removing the title testing as its not being set anymore), sending back to testing.

          Show
          Dan Poltawski added a comment - I've pushed a fix for this (removing the title testing as its not being set anymore), sending back to testing.
          Hide
          David Monllaó added a comment -

          It passes, tested in 23, 24 and master

          In 23 and 24 there is no modchoosertoggle-related stuff in course/renderer.php and add_modchoosertoodle() does not exists

          Show
          David Monllaó added a comment - It passes, tested in 23, 24 and master In 23 and 24 there is no modchoosertoggle-related stuff in course/renderer.php and add_modchoosertoodle() does not exists
          Hide
          Damyon Wiese added a comment -

          Congratulations! This issue has been resolved. Thanks for helping to make Moodle better for everyone!

          Regards, Damyon

          Show
          Damyon Wiese added a comment - Congratulations! This issue has been resolved. Thanks for helping to make Moodle better for everyone! Regards, Damyon

            People

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

              Dates

              • Created:
                Updated:
                Resolved: