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

Add maxlength rule in moodle form doesn't support utf8

    Details

    • Type: Bug
    • Status: Closed
    • Priority: 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 Master Branch:
      MDL-40267-master

      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

        Gliffy Diagrams

          Attachments

            Issue Links

              Activity

              Hide
              stronk7 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
              stronk7 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
              poltawski 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
              poltawski 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
              poltawski 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
              poltawski 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
              poltawski 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
              poltawski 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
              skodak Petr Skoda added a comment -

              +1

              Show
              skodak Petr Skoda added a comment - +1
              Hide
              marina Marina Glancy added a comment -

              Thanks Dan, integrated in 2.4, 2.5 and 2.6

              Show
              marina Marina Glancy added a comment - Thanks Dan, integrated in 2.4, 2.5 and 2.6
              Hide
              salvetore 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
              salvetore 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 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 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:
                    Fix Release Date:
                    11/Nov/13