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

          Attachments

            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