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
    • Rank:
      29131

      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.

        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 Škoda 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 Škoda 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 Škoda added a comment -

          reviewed, closing

          Show
          Petr Škoda added a comment - reviewed, closing

            People

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

              Dates

              • Created:
                Updated:
                Resolved: