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 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

        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 Skoda added a comment -

            +1

            Show
            Petr Skoda 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: