Details

    • Testing Instructions:
      Hide

      1/ install php-pspell extension and some dictionaries
      2/ configure TinyMCE to use PSpell extension and add spell checked languages for all your installed dictionaries
      3/ type some hebrew or any other language that uses non-ascii chars in editor
      4/ verify the invalid words are highlighted
      5/ verify suggestions do not contain double encoded chars

      Show
      1/ install php-pspell extension and some dictionaries 2/ configure TinyMCE to use PSpell extension and add spell checked languages for all your installed dictionaries 3/ type some hebrew or any other language that uses non-ascii chars in editor 4/ verify the invalid words are highlighted 5/ verify suggestions do not contain double encoded chars
    • Workaround:
      Hide

      Using the patch attached it worked like a charm.

      Show
      Using the patch attached it worked like a charm.
    • Affected Branches:
      MOODLE_21_STABLE, MOODLE_22_STABLE, MOODLE_23_STABLE
    • Fixed Branches:
      MOODLE_22_STABLE
    • Pull from Repository:
    • Pull Master Branch:
      w52_MDL-30953_m23_pspell

      Description

      The PSpell spell-checker plug-in of the tinymce editor doesn't behave well with utf-8, I tested this with Hebrew, at first it didn't give any results, after tinkering with it I arrived at the suggested patch attached to this issue.

        Gliffy Diagrams

        1. moodle-tinymce-pspell-he.patch
          2 kB
          David Hai Gootvilig
        1. pspell_error.png
          69 kB

          Activity

          Hide
          dhg David Hai Gootvilig added a comment -

          Hi Petr,

          Did you able to check this?

          --dhg

          Show
          dhg David Hai Gootvilig added a comment - Hi Petr, Did you able to check this? --dhg
          Hide
          skodak Petr Skoda added a comment -

          Thanks for the report and patch, I noticed utf-8 related problems in other spell options too. I am planning to work on it together with this issue later this week. Ideally we should submit our patches upstream.

          Petr

          Show
          skodak Petr Skoda added a comment - Thanks for the report and patch, I noticed utf-8 related problems in other spell options too. I am planning to work on it together with this issue later this week. Ideally we should submit our patches upstream. Petr
          Show
          skodak Petr Skoda added a comment - http://www.tinymce.com/forum/viewtopic.php?id=14326
          Hide
          skodak Petr Skoda added a comment - - edited

          I did my own testing and I have created a pull request with a slightly different patch, hopefully it will be accepted in upstream.

          https://github.com/tinymce/tinymce_spellchecker_php/pull/5

          Show
          skodak Petr Skoda added a comment - - edited I did my own testing and I have created a pull request with a slightly different patch, hopefully it will be accepted in upstream. https://github.com/tinymce/tinymce_spellchecker_php/pull/5
          Hide
          dhg David Hai Gootvilig added a comment -

          Thanx for all the good work.

          --dhg

          Show
          dhg David Hai Gootvilig added a comment - Thanx for all the good work. --dhg
          Hide
          dhg David Hai Gootvilig added a comment -
          Show
          dhg David Hai Gootvilig added a comment - My bad for deleting this... http://www.tinymce.com/develop/bugtracker_view.php?id=4926
          Hide
          skodak Petr Skoda added a comment -

          To integrators: please cherry pick to 2.2.x and 2.1.x, this change does not need full tinymce rebuild because only PHP files are affected.

          Show
          skodak Petr Skoda added a comment - To integrators: please cherry pick to 2.2.x and 2.1.x, this change does not need full tinymce rebuild because only PHP files are affected.
          Hide
          skodak Petr Skoda added a comment -

          Thanks a lot for your report and proposed patch, I suppose it will be fixed in the next weekly build.

          Ciao

          Show
          skodak Petr Skoda added a comment - Thanks a lot for your report and proposed patch, I suppose it will be fixed in the next weekly build. Ciao
          Hide
          stronk7 Eloy Lafuente (stronk7) added a comment -

          Note this applies ok to 22_STABLE and master, but not to 21_STABLE. We are using 3.4.2 there. So it requires separated patch. Halting till then. Ciao

          Show
          stronk7 Eloy Lafuente (stronk7) added a comment - Note this applies ok to 22_STABLE and master, but not to 21_STABLE. We are using 3.4.2 there. So it requires separated patch. Halting till then. Ciao
          Hide
          skodak Petr Skoda added a comment -

          ah, in that case please cherry pick to 2.2.x only, it will be part of tinymce if we ever decide to backport it to 2.1.x

          Show
          skodak Petr Skoda added a comment - ah, in that case please cherry pick to 2.2.x only, it will be part of tinymce if we ever decide to backport it to 2.1.x
          Hide
          stronk7 Eloy Lafuente (stronk7) added a comment -

          Integrated, thanks! (22 and master only).

          Show
          stronk7 Eloy Lafuente (stronk7) added a comment - Integrated, thanks! (22 and master only).
          Hide
          gerry Gerard Caulfield added a comment -

          When clicking to see a spelling correction I get the following error which it seems is from a throw made in the _getPLink() function just below the patch.

          Also as you can also see in the screenshot only one of the sets of Hebrew characters are underlined, although this is probably due to the first issue.

          Show
          gerry Gerard Caulfield added a comment - When clicking to see a spelling correction I get the following error which it seems is from a throw made in the _getPLink() function just below the patch. Also as you can also see in the screenshot only one of the sets of Hebrew characters are underlined, although this is probably due to the first issue.
          Hide
          skodak Petr Skoda added a comment -

          Hmm, this kind of error most probably indicates problem in your install, not the PHP code. Please make sure your pspell PHP extension works before failing this test please.

          To integrators: please restart testing of this issue, I do not think there is any bug in the code itself, it works fine here. And if there is a problem it is a different issue - not utf-8 related.

          Show
          skodak Petr Skoda added a comment - Hmm, this kind of error most probably indicates problem in your install, not the PHP code. Please make sure your pspell PHP extension works before failing this test please. To integrators: please restart testing of this issue, I do not think there is any bug in the code itself, it works fine here. And if there is a problem it is a different issue - not utf-8 related.
          Hide
          gerry Gerard Caulfield added a comment -

          I don't know what it was but I can no longer produce the bug shown in the screenshot and everything seems to be working perfectly. sigh

          Show
          gerry Gerard Caulfield added a comment - I don't know what it was but I can no longer produce the bug shown in the screenshot and everything seems to be working perfectly. sigh
          Hide
          skodak Petr Skoda added a comment -

          process restarted, thanks

          Show
          skodak Petr Skoda added a comment - process restarted, thanks
          Hide
          gerry Gerard Caulfield added a comment -

          Passed, thanks

          Show
          gerry Gerard Caulfield added a comment - Passed, thanks
          Hide
          salvetore Michael de Raadt added a comment -

          Thanks for working in this, guys.

          I've created a sub-task to backport a fix to 2.1.

          Show
          salvetore Michael de Raadt added a comment - Thanks for working in this, guys. I've created a sub-task to backport a fix to 2.1.
          Hide
          dhg David Hai Gootvilig added a comment -

          Thanx everybody for your work on this.

          I am not sure if this is ok to ask. but could you also look at MDL-30756?

          Show
          dhg David Hai Gootvilig added a comment - Thanx everybody for your work on this. I am not sure if this is ok to ask. but could you also look at MDL-30756 ?
          Hide
          stronk7 Eloy Lafuente (stronk7) added a comment -

          The git and cvs repositories are happy receiving your very first contribution to Moodle for 2012. Happy new year!

          Closing, ciao

          Show
          stronk7 Eloy Lafuente (stronk7) added a comment - The git and cvs repositories are happy receiving your very first contribution to Moodle for 2012. Happy new year! Closing, ciao

            People

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

              Dates

              • Created:
                Updated:
                Resolved:
                Fix Release Date:
                9/Jan/12