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 Master Branch:

      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.

        Gliffy Diagrams

          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: