Moodle
  1. Moodle
  2. MDL-40267

Add maxlength rule in moodle form doesn't support utf8

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Critical Critical
    • Resolution: Fixed
    • Affects Version/s: 2.4, 2.4.6, 2.5.2
    • Fix Version/s: 2.4.7, 2.5.3, 2.6
    • Component/s: Forms Library
    • Labels:
    • Testing Instructions:
      Hide

      1/ Run phpunit tests on all branches
      2/ Upload the attached test.php to your moodle, and run it in the browser.
      3/ Fill our the form fields (in order)

      A: 日本語語
      B: 日本語語
      C: 日本語
      D: 日本語
      

      4/ VERIFY: you get a JS warning about A and C
      5/ Change the fields with warnings to:

      A: 日本語
      C: 日本語語
      

      6/ VERIFY: the JS warnings go away
      7/ Submit the form
      8/ VERIFY: warnings about B and D are seen
      9/ VERIFY: 'null' is displayed at the top
      10/ Change the fields with warnigns to:

      B: 日本語
      D: 日本語日
      

      11/ Submit the form
      12/ VERIFY: the form submits
      13/ VERIFY: you see a var_dump of all the expected data

      Show
      1/ Run phpunit tests on all branches 2/ Upload the attached test.php to your moodle, and run it in the browser. 3/ Fill our the form fields (in order) A: 日本語語 B: 日本語語 C: 日本語 D: 日本語 4/ VERIFY: you get a JS warning about A and C 5/ Change the fields with warnings to: A: 日本語 C: 日本語語 6/ VERIFY: the JS warnings go away 7/ Submit the form 8/ VERIFY: warnings about B and D are seen 9/ VERIFY: 'null' is displayed at the top 10/ Change the fields with warnigns to: B: 日本語 D: 日本語日 11/ Submit the form 12/ VERIFY: the form submits 13/ VERIFY: you see a var_dump of all the expected data
    • Affected Branches:
      MOODLE_24_STABLE, MOODLE_25_STABLE
    • Fixed Branches:
      MOODLE_24_STABLE, MOODLE_25_STABLE, MOODLE_26_STABLE
    • Pull 2.4 Branch:
    • Pull 2.5 Branch:
    • Pull Master Branch:
      MDL-40267-master
    • Rank:
      51035

      Description

      When using maxlength rule on moodle form with utf8 lang
      you can actually add half of the chars in the rule for example in most of the activities mod_form
      the name max length is 255 but in utf8 you can add only 127 chars

      $mform->addRule('name', get_string('maximumchars', '', 255), 'maxlength', 255, 'client');

      It's happened because pear HTML_QuickForm Rule_Range class use strlen instead of using mb_strlen and encoding format

      1. test.php
        1 kB
        Dan Poltawski

        Issue Links

          Activity

          Hide
          Eloy Lafuente (stronk7) added a comment -

          Raising priority of this. We have dozens of 'maxlength' validations that are behaving incorrectly by counting bytes instead of chars.

          Note I arrived here from MDL-42270, where we are adding one more rule, knowing it's affected by this.

          Ciao

          Show
          Eloy Lafuente (stronk7) added a comment - Raising priority of this. We have dozens of 'maxlength' validations that are behaving incorrectly by counting bytes instead of chars. Note I arrived here from MDL-42270 , where we are adding one more rule, knowing it's affected by this. Ciao
          Hide
          Dan Poltawski added a comment -

          Looks to me like the client-side rule is working perfectly. (Javascript .length handling utf8 fine).

          So, its a very simple fix to hack the library (many of those already).

          I'm going to spend a bit of time:
          1) Writing unit tests
          2) Writing behat test for the JS validator.

          Show
          Dan Poltawski added a comment - Looks to me like the client-side rule is working perfectly. (Javascript .length handling utf8 fine). So, its a very simple fix to hack the library (many of those already). I'm going to spend a bit of time: 1) Writing unit tests 2) Writing behat test for the JS validator.
          Hide
          Dan Poltawski added a comment -

          Here is a test form.

          For the client side:
          Note that if you type 日本語 you'll not get a JS validation warning.

          If you type 日本語本, you will get a JS validation warning.

          For the server side, its failing for 日本語.

          Show
          Dan Poltawski added a comment - Here is a test form. For the client side: Note that if you type 日本語 you'll not get a JS validation warning. If you type 日本語本, you will get a JS validation warning. For the server side, its failing for 日本語.
          Hide
          Dan Poltawski added a comment - - edited

          Requesting peer review.

          I wanted to use the forms mock_submit stuff to test this. But it appears that is not working. Creating a new issue about that.

          Show
          Dan Poltawski added a comment - - edited Requesting peer review. I wanted to use the forms mock_submit stuff to test this. But it appears that is not working. Creating a new issue about that.
          Hide
          Petr Škoda added a comment -

          +1

          Show
          Petr Škoda added a comment - +1
          Hide
          Marina Glancy added a comment -

          Thanks Dan, integrated in 2.4, 2.5 and 2.6

          Show
          Marina Glancy added a comment - Thanks Dan, integrated in 2.4, 2.5 and 2.6
          Hide
          Michael de Raadt added a comment -

          Test result: Success!

          Tested in 2.4, 2.5 and master.

          Unit tests passed.

          I tested using the given form with the prescribed inputs as well as a mixture of wide and single-byte characters.

          Show
          Michael de Raadt added a comment - Test result: Success! Tested in 2.4, 2.5 and master. Unit tests passed. I tested using the given form with the prescribed inputs as well as a mixture of wide and single-byte characters.
          Hide
          Damyon Wiese added a comment -

          Here lies 52 bugs.
          All fixed or swept under a rug.
          If they come back one day,
          To our dismay,
          We all will feel quite un-smug.

          Thanks for the reporting/fixing/testing on this issue. It has been sent upstream.

          Show
          Damyon Wiese added a comment - Here lies 52 bugs. All fixed or swept under a rug. If they come back one day, To our dismay, We all will feel quite un-smug. Thanks for the reporting/fixing/testing on this issue. It has been sent upstream.

            People

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

              Dates

              • Created:
                Updated:
                Resolved: