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

          Attachments

            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 Mudrák 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 Mudrák 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 Mudrák 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 Mudrák 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 Mudrák 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 Mudrák 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