Moodle
  1. Moodle
  2. MDL-22174

print_group_picture creates empty group links

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 1.9.8
    • Fix Version/s: 1.9.9
    • Component/s: Libraries
    • Labels:
      None
    • Affected Branches:
      MOODLE_19_STABLE
    • Fixed Branches:
      MOODLE_19_STABLE
    • Rank:
      26000

      Description

      print_group_picture prints group links even when there is no picture for the group, so the link is like <a ...></a> - this creates an invisible tabstop which is confusing for screenreader and other keyboard users, as well as wasting a few bytes.

      To reproduce, do the following:

      1) create course, add yourself as teacher
      2) create 3 groups (these are all 3 possible cases with regard to group pictures):
      a) with a picture
      b) with a picture but with 'hide picture'
      c) with no picture
      3) create forum, start discussion

      • observe that if you focus the user name at top of the post and press tab, it tabs through the 2 pictures but there is then an 'invisible' tab spot
      • if you log in as a student, the 'hide picture' option does work, apparently to remove both the picture AND the link, i.e. a group with hide picture is better than one with no picture at all from that perspective. Presumably it is intentional that 'hide picture' does not work for users those with managegroups capability i.e. it really means 'hide picture from students'. (how DO you get rid of a group picture once you've made one? ho hum)

      I propose making it so that if a group has no picture, then the output from print_group_picture for that group will be an empty string and not a link with empty content.

        Activity

        Hide
        Sam Marshall added a comment -

        Here's the patch for 1.9. I tested this using the procedure described earlier, it produces the same visual results but there is now no hidden link.

        Show
        Sam Marshall added a comment - Here's the patch for 1.9. I tested this using the procedure described earlier, it produces the same visual results but there is now no hidden link.
        Hide
        Sam Marshall added a comment -

        patch also applies to HEAD (yay)

        Show
        Sam Marshall added a comment - patch also applies to HEAD (yay)
        Hide
        Petr Škoda added a comment -

        +1 for commit after fixing trailing whitespace

        Show
        Petr Škoda added a comment - +1 for commit after fixing trailing whitespace
        Hide
        Sam Marshall added a comment -

        thanks, committed to MOODLE_19 and HEAD

        Show
        Sam Marshall added a comment - thanks, committed to MOODLE_19 and HEAD

          People

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

            Dates

            • Created:
              Updated:
              Resolved: