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

Accessibility: guest access with and without password icons alt text identical

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: 2.4.7, 2.5.3, 2.6
    • Fix Version/s: 2.4.8, 2.5.4, 2.6.1
    • Component/s: Accessibility
    • Labels:
    • Testing Instructions:
      Hide

      Create two courses, both with guest access set to Yes. For the second course, set a key.

      View the category list (course/index.php?categoryid=x) You should see that both courses have a guest access icon which looks in the standard theme like a person's head. The second course with the key set should include a small key in the icon.

      Hover over both icons (or inspect in firebug). Note that both have the same alt and title text "Guest Access".

      Apply the patch, then look at the alt and title text for both icons again. The course with the key should now have different alt & title text indicating that a key is required.

      Show
      Create two courses, both with guest access set to Yes. For the second course, set a key. View the category list (course/index.php?categoryid=x) You should see that both courses have a guest access icon which looks in the standard theme like a person's head. The second course with the key set should include a small key in the icon. Hover over both icons (or inspect in firebug). Note that both have the same alt and title text "Guest Access". Apply the patch, then look at the alt and title text for both icons again. The course with the key should now have different alt & title text indicating that a key is required.
    • Affected Branches:
      MOODLE_24_STABLE, MOODLE_25_STABLE, MOODLE_26_STABLE
    • Fixed Branches:
      MOODLE_24_STABLE, MOODLE_25_STABLE, MOODLE_26_STABLE
    • Pull from Repository:
    • Pull Master Branch:
      wip-MDL-43135

      Description

      There are two versions of the guest access icon displayed in course listings, one used when a course has guest access with an enrolment key and one without.

      However, the alt text assigned to both icons is the same. It ought to be different to clarify things for visually impaired users.

        Gliffy Diagrams

          Activity

          Hide
          jenny-gray Jenny Gray added a comment - - edited

          On a perhaps related note, there's an opentoguests string in moodle.php lang pack which isn't used anywhere in the system and should probably be removed.

          Show
          jenny-gray Jenny Gray added a comment - - edited On a perhaps related note, there's an opentoguests string in moodle.php lang pack which isn't used anywhere in the system and should probably be removed.
          Hide
          salvetore Michael de Raadt added a comment -

          Thanks for reporting this, Jenny.

          Feel free to work on this issue.

          Show
          salvetore Michael de Raadt added a comment - Thanks for reporting this, Jenny. Feel free to work on this issue.
          Hide
          marina Marina Glancy added a comment - - edited

          Hi Jenny, this looks good, thanks for working on it.
          Can we please not remove unused string from stable versions and remove it from master only?
          Also 'pluginname_withkey' is not a good name for the string. Better using 'guestaccess_withkey' or something like that. 'pluginname' is a name of the plugin as it appears in all plugin listings, it just happens so that it is used for icon alt (which is not extremely good too but that's how it is)
          Thanks again

          Show
          marina Marina Glancy added a comment - - edited Hi Jenny, this looks good, thanks for working on it. Can we please not remove unused string from stable versions and remove it from master only? Also 'pluginname_withkey' is not a good name for the string. Better using 'guestaccess_withkey' or something like that. 'pluginname' is a name of the plugin as it appears in all plugin listings, it just happens so that it is used for icon alt (which is not extremely good too but that's how it is) Thanks again
          Hide
          jenny-gray Jenny Gray added a comment -

          Thanks for the quick review Marina. You picked exactly the two things to comment on that I thought might cause concern.

          I'll fix them up and submit for integration.

          Show
          jenny-gray Jenny Gray added a comment - Thanks for the quick review Marina. You picked exactly the two things to comment on that I thought might cause concern. I'll fix them up and submit for integration.
          Hide
          jenny-gray Jenny Gray added a comment -

          Peer review comments from Marina incorporated, and commit messages improved.

          Show
          jenny-gray Jenny Gray added a comment - Peer review comments from Marina incorporated, and commit messages improved.
          Hide
          marina Marina Glancy added a comment -

          Thanks Jenny, looks good.
          Can you review your 2.4 branch - looks like there are much more commits there than needed
          Thanks!

          Show
          marina Marina Glancy added a comment - Thanks Jenny, looks good. Can you review your 2.4 branch - looks like there are much more commits there than needed Thanks!
          Hide
          jenny-gray Jenny Gray added a comment -

          Hm, old work creeping in. Not sure how that happened. Sorted.

          Show
          jenny-gray Jenny Gray added a comment - Hm, old work creeping in. Not sure how that happened. Sorted.
          Hide
          poltawski Dan Poltawski 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
          poltawski Dan Poltawski 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
          stronk7 Eloy Lafuente (stronk7) added a comment -

          Just guessing if it wouldn't be better to:

          1) Name the lang string "guestaccess_withpassword", just to keep it "paired" with the icon name.
          2) Also I'd suggest to move from currently not perfect "pluginname" use to a new (AMOS COPY) "guestaccess_withoutpassword" lang string. I can imagine langs where the "pluginame" is not the best string to show information from.

          I'll keep this open for some hours, waiting for feedback... ciao

          Show
          stronk7 Eloy Lafuente (stronk7) added a comment - Just guessing if it wouldn't be better to: 1) Name the lang string "guestaccess_withpassword", just to keep it "paired" with the icon name. 2) Also I'd suggest to move from currently not perfect "pluginname" use to a new (AMOS COPY) "guestaccess_withoutpassword" lang string. I can imagine langs where the "pluginame" is not the best string to show information from. I'll keep this open for some hours, waiting for feedback... ciao
          Hide
          jenny-gray Jenny Gray added a comment -

          Hi Eloy, I can make those changes now for you if you wish. They seem reasonable, I was only trying to keep lang strings short and to change as little as possible.

          Can you explain what you mean by (AMOS COPY) though? Do you just want a patch something like:

          +$string['guestaccess_withpassword'] = 'Guest access requires password';
          +$string['guestaccess_withoutpassword'] = 'Guest access';

          Or is there extra information that AMOS requires?

          Show
          jenny-gray Jenny Gray added a comment - Hi Eloy, I can make those changes now for you if you wish. They seem reasonable, I was only trying to keep lang strings short and to change as little as possible. Can you explain what you mean by (AMOS COPY) though? Do you just want a patch something like: +$string ['guestaccess_withpassword'] = 'Guest access requires password'; +$string ['guestaccess_withoutpassword'] = 'Guest access'; Or is there extra information that AMOS requires?
          Hide
          stronk7 Eloy Lafuente (stronk7) added a comment - - edited

          In order to get all languages "filled" as soon as possible, especially in stable branches that are already in production.... when creating new lang strings you can specify, in the commit message the introduces the changes for "en", how they can be created and filled by default in all the other languages.

          So adding something like this in the commit text (after 1st line + blank line):

          AMOS BEGIN
           CPY [pluginname,enrol_guest],[guestaccess_withpassword,enrol_guest]
           CPY [pluginname,enrol_guest],[guestaccess_withoutpassword,enrol_guest]
          AMOS END
          

          Will help all languages to get the (two) new strings created and populated with the contents originally available @ "pluginname". Later translators can tidy up the new strings if decided.

          Adding David Mudrak and Helen Foster here to confirm it's correct to copy that way and the default lang strings are ok, if we proceed to split the old "pluginname" uses to 2 new strings.

          Info:

          Show
          stronk7 Eloy Lafuente (stronk7) added a comment - - edited In order to get all languages "filled" as soon as possible, especially in stable branches that are already in production.... when creating new lang strings you can specify, in the commit message the introduces the changes for "en", how they can be created and filled by default in all the other languages. So adding something like this in the commit text (after 1st line + blank line): AMOS BEGIN CPY [pluginname,enrol_guest],[guestaccess_withpassword,enrol_guest] CPY [pluginname,enrol_guest],[guestaccess_withoutpassword,enrol_guest] AMOS END Will help all languages to get the (two) new strings created and populated with the contents originally available @ "pluginname". Later translators can tidy up the new strings if decided. Adding David Mudrak and Helen Foster here to confirm it's correct to copy that way and the default lang strings are ok, if we proceed to split the old "pluginname" uses to 2 new strings. Info: http://docs.moodle.org/dev/Languages/AMOS#AMOS_script http://docs.moodle.org/dev/Commit_cheat_sheet#Include_AMOS_script_in_the_commit_if_needed
          Hide
          tsala Helen Foster added a comment -

          Eloy's suggested solution seems good to me. I think the default lang strings are OK.

          Show
          tsala Helen Foster added a comment - Eloy's suggested solution seems good to me. I think the default lang strings are OK.
          Hide
          mudrd8mz David Mudrak added a comment -

          Just to highlight the bit documented in the linked docs - the AMOScript must be put into the commit message of a commit that itself modifies some English strings file. AMOS tracks all commits that somehow modify some /lang/en/ file and searches for the AMOScript in them.

          In this particular case, either of e1c9cb9 and f0f5a3d can be used as both modify some language file.

          Show
          mudrd8mz David Mudrak added a comment - Just to highlight the bit documented in the linked docs - the AMOScript must be put into the commit message of a commit that itself modifies some English strings file. AMOS tracks all commits that somehow modify some /lang/en/ file and searches for the AMOScript in them. In this particular case, either of e1c9cb9 and f0f5a3d can be used as both modify some language file.
          Hide
          stronk7 Eloy Lafuente (stronk7) added a comment -

          Thanks Helen and David!

          I'll keep this open some more time, Jenny, let me know if you prefer reopening it or need any assistance.

          Ciao

          Show
          stronk7 Eloy Lafuente (stronk7) added a comment - Thanks Helen and David! I'll keep this open some more time, Jenny, let me know if you prefer reopening it or need any assistance. Ciao
          Hide
          jenny-gray Jenny Gray added a comment -

          Thanks all - you learn something new every day! Eloy, I've got a system to upgrade first thing this morning but should be able to get this done for you in a few hours time.

          Show
          jenny-gray Jenny Gray added a comment - Thanks all - you learn something new every day! Eloy, I've got a system to upgrade first thing this morning but should be able to get this done for you in a few hours time.
          Hide
          jenny-gray Jenny Gray added a comment -

          OK Eloy, those changes should be there now.

          Show
          jenny-gray Jenny Gray added a comment - OK Eloy, those changes should be there now.
          Hide
          stronk7 Eloy Lafuente (stronk7) added a comment -

          Thanks, Jenny. They look perfect!

          Integrating them in some minutes...

          Show
          stronk7 Eloy Lafuente (stronk7) added a comment - Thanks, Jenny. They look perfect! Integrating them in some minutes...
          Hide
          stronk7 Eloy Lafuente (stronk7) added a comment -

          Integrated (24, 25, 26 & master), thanks!

          Show
          stronk7 Eloy Lafuente (stronk7) added a comment - Integrated (24, 25, 26 & master), thanks!
          Hide
          poltawski Dan Poltawski added a comment -

          Thanks Jennie, Passing. Looks good on master, 26, 25 and 24.

          Show
          poltawski Dan Poltawski added a comment - Thanks Jennie, Passing. Looks good on master, 26, 25 and 24.
          Hide
          samhemelryk Sam Hemelryk added a comment -

          Thanks for the code, its now upstream!

          Heres a fun trick to try in the spirit of Friday the 13th.
          I hear if you stand in front a mirror, alone, in the dark, and say "Oracle" three times Petr Skoka will appear in the mirror and you'll see him deleting the Oracle driver from Moodle.

          Show
          samhemelryk Sam Hemelryk added a comment - Thanks for the code, its now upstream! Heres a fun trick to try in the spirit of Friday the 13th. I hear if you stand in front a mirror, alone, in the dark, and say "Oracle" three times Petr Skoka will appear in the mirror and you'll see him deleting the Oracle driver from Moodle.
          Hide
          mudrd8mz David Mudrak added a comment -

          Thanks for the AMOScript provided Jenny! I can confirm it was executed correctly and as expected. Translators would love you, if they knew what happened. But they don't. And that's good, because that is why we have AMOScript

          Show
          mudrd8mz David Mudrak added a comment - Thanks for the AMOScript provided Jenny! I can confirm it was executed correctly and as expected. Translators would love you, if they knew what happened. But they don't. And that's good, because that is why we have AMOScript
          Hide
          jenny-gray Jenny Gray added a comment -

          Brilliant - thanks all. Now to back-port to OpenLearn

          Show
          jenny-gray Jenny Gray added a comment - Brilliant - thanks all. Now to back-port to OpenLearn

            People

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

              Dates

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