Moodle
  1. Moodle
  2. MDL-8592

HTML editor spell checker won't work on edit profile page referenced through https

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 1.6.4, 1.7.1
    • Fix Version/s: 1.6.7, 1.7.5, 1.8.6, 1.9.1
    • Component/s: HTML Editor (TinyMCE)
    • Labels:
      None
    • Environment:
      RHEL 4 and Solaris 10
    • Database:
      MySQL
    • Affected Branches:
      MOODLE_16_STABLE, MOODLE_17_STABLE
    • Fixed Branches:
      MOODLE_16_STABLE, MOODLE_17_STABLE, MOODLE_18_STABLE, MOODLE_19_STABLE

      Description

      If you use the spell checker from a page loaded with https, e.g. the edit profile page, the spell checker box pops up but the text to spell check is never loaded. If you have loginhttps set to yes, the default loading of the edit profile page will be through https, though you can manually change it to http:// and then the spell checker works fine.

        Gliffy Diagrams

          Issue Links

            Activity

            Hide
            Ann Adamcik added a comment -

            Note that none of the editor functions that bring up a dialog or external window will work on the profile page when using https (e.g. font color, bg color, anchor, link, image, etc).

            In each case, there is a javascript error - uncaught exception: Permission denied to get property Window.Dialog (or Window.speller, or Window.HTMLArea).

            Show
            Ann Adamcik added a comment - Note that none of the editor functions that bring up a dialog or external window will work on the profile page when using https (e.g. font color, bg color, anchor, link, image, etc). In each case, there is a javascript error - uncaught exception: Permission denied to get property Window.Dialog (or Window.speller, or Window.HTMLArea).
            Hide
            Anthony Borrow added a comment -

            I wonder if we can make use of the httpsrequired() function by checking if the global variable $HTTPSPAGEREQUIRED is set. For the spell check, I was able to get it to work but not without breaking the regular pages. The key section of code is:

            /lib/weblib.php in the print_speller_code function.

            I thought of doing something like:

            function print_speller_code ($usehtmleditor=false, $return=false) {
            global $CFG;
            $str = '';
            if ($HTTPSPAGEREQUIRED == true)

            { $wwwroot=$CFG->httpswwwroot; }

            else

            { $wwwroot = $CFG->wwwroot; }

            if(!$usehtmleditor) {
            $str .= 'function openSpellChecker()

            {'."\n"; $str .= "\tvar speller = new spellChecker();\n"; $str .= "\tspeller.popUpUrl = \"" . $wwwroot ."/lib/speller/spellchecker.html\";\n"; $str .= "\tspeller.spellCheckScript = \"". $wwwroot ."/lib/speller/server-scripts/spellchecker.php\";\n"; $str .= "\tspeller.spellCheckAll();\n"; $str .= '}

            '."\n";
            } else {
            $str .= "function spellClickHandler(editor, buttonId)

            {\n"; $str .= "\teditor._textArea.value = editor.getHTML();\n"; $str .= "\tvar speller = new spellChecker( editor._textArea );\n"; $str .= "\tspeller.popUpUrl = \"" . $wwwroot ."/lib/speller/spellchecker.html\";\n"; $str .= "\tspeller.spellCheckScript = \"". $wwwroot ."/lib/speller/server-scripts/spellchecker.php\";\n"; $str .= "\tspeller._moogle_edit=1;\n"; $str .= "\tspeller._editor=editor;\n"; $str .= "\tspeller.openChecker();\n"; $str .= '}

            '."\n";
            }

            So that the popUpURL and spellCheckScript would be https when the calling page requires it and otherwise stick with $CFG->wwwroot. We may need to pass another variable through the chain to indicate what is happening. I was hoping since it was a global that it would remain set but I'm too tired to follow it through right now. I hope this helps point you in the right direction though.

            I suspect similar issues are happening with what Ann mentions above. Let me know if you have questions.

            Peace - Anthony

            Show
            Anthony Borrow added a comment - I wonder if we can make use of the httpsrequired() function by checking if the global variable $HTTPSPAGEREQUIRED is set. For the spell check, I was able to get it to work but not without breaking the regular pages. The key section of code is: /lib/weblib.php in the print_speller_code function. I thought of doing something like: function print_speller_code ($usehtmleditor=false, $return=false) { global $CFG; $str = ''; if ($HTTPSPAGEREQUIRED == true) { $wwwroot=$CFG->httpswwwroot; } else { $wwwroot = $CFG->wwwroot; } if(!$usehtmleditor) { $str .= 'function openSpellChecker() {'."\n"; $str .= "\tvar speller = new spellChecker();\n"; $str .= "\tspeller.popUpUrl = \"" . $wwwroot ."/lib/speller/spellchecker.html\";\n"; $str .= "\tspeller.spellCheckScript = \"". $wwwroot ."/lib/speller/server-scripts/spellchecker.php\";\n"; $str .= "\tspeller.spellCheckAll();\n"; $str .= '} '."\n"; } else { $str .= "function spellClickHandler(editor, buttonId) {\n"; $str .= "\teditor._textArea.value = editor.getHTML();\n"; $str .= "\tvar speller = new spellChecker( editor._textArea );\n"; $str .= "\tspeller.popUpUrl = \"" . $wwwroot ."/lib/speller/spellchecker.html\";\n"; $str .= "\tspeller.spellCheckScript = \"". $wwwroot ."/lib/speller/server-scripts/spellchecker.php\";\n"; $str .= "\tspeller._moogle_edit=1;\n"; $str .= "\tspeller._editor=editor;\n"; $str .= "\tspeller.openChecker();\n"; $str .= '} '."\n"; } So that the popUpURL and spellCheckScript would be https when the calling page requires it and otherwise stick with $CFG->wwwroot. We may need to pass another variable through the chain to indicate what is happening. I was hoping since it was a global that it would remain set but I'm too tired to follow it through right now. I hope this helps point you in the right direction though. I suspect similar issues are happening with what Ann mentions above. Let me know if you have questions. Peace - Anthony
            Hide
            Anthony Borrow added a comment -

            Petr - While this is not the most pressing of bugs, the issue strikes me as one that could have tentacles into a number of other issues so I have added you to the list of watchers to this issue as something to look at after 1.9 is released (I know you have your hands full with the remaining critical issues). The issue here has to do with the use of the httpsrequired function and ensuring that the related links like the spell checker, themes, etc. also use https. Peace - Anthony

            Show
            Anthony Borrow added a comment - Petr - While this is not the most pressing of bugs, the issue strikes me as one that could have tentacles into a number of other issues so I have added you to the list of watchers to this issue as something to look at after 1.9 is released (I know you have your hands full with the remaining critical issues). The issue here has to do with the use of the httpsrequired function and ensuring that the related links like the spell checker, themes, etc. also use https. Peace - Anthony
            Hide
            Anthony Borrow added a comment -

            For those looking for a quick and easy workaround (although granted perhaps a somewhat expensive one), you can change the config.php file so that wwwroot uses the https protocol. In other words, change:

            $CFG->wwwroot='http://yourmoodle.com';

            to:

            CFG->wwwroot='https://yourmoodle.com'

            The spell checking will then work at the expense of every page being encrypted as https. This should also work around the issues that Ann raises too. Just be aware that doing so places a little more processor demand on the server. Peace - Anthony

            Show
            Anthony Borrow added a comment - For those looking for a quick and easy workaround (although granted perhaps a somewhat expensive one), you can change the config.php file so that wwwroot uses the https protocol. In other words, change: $CFG->wwwroot='http://yourmoodle.com'; to: CFG->wwwroot='https://yourmoodle.com' The spell checking will then work at the expense of every page being encrypted as https. This should also work around the issues that Ann raises too. Just be aware that doing so places a little more processor demand on the server. Peace - Anthony
            Hide
            Petr Skoda added a comment -

            well, I tried to fix this about a year ago, the problem was that the editor does not know it is on page requiring https, we could pass it to editor, but then it would have to pass it to other scripts written in JS which is main problem imho

            I am afraid this will have to wait till new editor is introduced in 2.0

            Show
            Petr Skoda added a comment - well, I tried to fix this about a year ago, the problem was that the editor does not know it is on page requiring https, we could pass it to editor, but then it would have to pass it to other scripts written in JS which is main problem imho I am afraid this will have to wait till new editor is introduced in 2.0
            Hide
            Martin Dougiamas added a comment -

            Bumping to 2.0

            Show
            Martin Dougiamas added a comment - Bumping to 2.0
            Hide
            Mathieu Petit-Clair added a comment - - edited

            Here's a patch (against 19_STABLE) that fixes this. Works for me, with (profile editing) and without (blog editing) https. This obviously needs testing and review...

            This could be a nice occasion to do some refactoring: what looks like the same code is in three different places. The code in weblib.php, function print_speller_code() seems to be a bit more complex than the one in /lib/speller/lib.php, function speller_enable().

            Show
            Mathieu Petit-Clair added a comment - - edited Here's a patch (against 19_STABLE) that fixes this. Works for me, with (profile editing) and without (blog editing) https. This obviously needs testing and review... This could be a nice occasion to do some refactoring: what looks like the same code is in three different places. The code in weblib.php, function print_speller_code() seems to be a bit more complex than the one in /lib/speller/lib.php, function speller_enable().
            Hide
            Anthony Borrow added a comment -

            Mathiew - What is the value of $CFG->httpswwwroot when there is not ssl enabled. I just want to make sure that we are not breaking things. If that case is already handled then it should be fine. Does that make sense? Also, since you are in this area of the code you may want to check out MDL-14590 which has to do with messaging. I'll try to do some testing on this patch and see how it works. Peace - Anthony

            Show
            Anthony Borrow added a comment - Mathiew - What is the value of $CFG->httpswwwroot when there is not ssl enabled. I just want to make sure that we are not breaking things. If that case is already handled then it should be fine. Does that make sense? Also, since you are in this area of the code you may want to check out MDL-14590 which has to do with messaging. I'll try to do some testing on this patch and see how it works. Peace - Anthony
            Hide
            Mathieu Petit-Clair added a comment -

            I just looked at the code, moodlelib.php (around line 6306) ... if loginhttps is disabled, then httpswwwroot and httpsthemewww have the value of their non-ssl equivalent.

            I closed MDL-14590 a few minutes ago.

            Show
            Mathieu Petit-Clair added a comment - I just looked at the code, moodlelib.php (around line 6306) ... if loginhttps is disabled, then httpswwwroot and httpsthemewww have the value of their non-ssl equivalent. I closed MDL-14590 a few minutes ago.
            Hide
            Anthony Borrow added a comment -

            I saw the commit for MDL-14590 and tested it. I've tested the patch you included and it seems to resolve this issue so my +1 for resolving it. Peace - Anthony

            Show
            Anthony Borrow added a comment - I saw the commit for MDL-14590 and tested it. I've tested the patch you included and it seems to resolve this issue so my +1 for resolving it. Peace - Anthony
            Hide
            Anthony Borrow added a comment -

            The patch looked pretty straight forward and simple, do you want to merge it with some of the previous branches (since it was originally reported as affecting 1.6 and 1.7)?

            Show
            Anthony Borrow added a comment - The patch looked pretty straight forward and simple, do you want to merge it with some of the previous branches (since it was originally reported as affecting 1.6 and 1.7)?
            Hide
            Mathieu Petit-Clair added a comment -

            We could backport this to older versions, but for 2.0 we should really clean up the code duplication, or make the reason behind it a bit more obvious.

            Show
            Mathieu Petit-Clair added a comment - We could backport this to older versions, but for 2.0 we should really clean up the code duplication, or make the reason behind it a bit more obvious.
            Hide
            Anthony Borrow added a comment -

            I agree - backporting would be good and then cleaning up the code for 2.0 would be great - I can't think of a good reason for the duplication. Just going off the patch, I would say keep it as localized as possible - so I would think /lib/speller/lib.php would be the best one to keep. Peace - Anthony

            Show
            Anthony Borrow added a comment - I agree - backporting would be good and then cleaning up the code for 2.0 would be great - I can't think of a good reason for the duplication. Just going off the patch, I would say keep it as localized as possible - so I would think /lib/speller/lib.php would be the best one to keep. Peace - Anthony
            Hide
            Mathieu Petit-Clair added a comment -

            Fixed in all versions.

            Show
            Mathieu Petit-Clair added a comment - Fixed in all versions.
            Hide
            Petr Skoda added a comment -

            reviewed, closing

            Show
            Petr Skoda added a comment - reviewed, closing

              People

              • Votes:
                7 Vote for this issue
                Watchers:
                3 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved: