moodleou / moodle-mod_ouwiki

Alternative wiki module for Moodle 2 (designed for use in teaching and learning)
36 stars 32 forks source link

Double spaces issue in text in Ouwiki 2.X #9

Closed frankgasking closed 11 years ago

frankgasking commented 12 years ago

There was a bug in the ou wiki tool on 1.9 that meant that when you put a double space in the text, any content after it would not be saved. It seems to have carried over also into 2.X

To recreate the error, in the main content area when creating a new wiki entry, if you put a double space (hitting spacebar twice) within that text area it produces that error (it also happens if there is a space at the beginning of a new line!). A lot of people are taught to double space after a full stop (particularly touch typists) so it’s quite a major flaw and seems to have carried over between versions.

frankgasking commented 12 years ago

Here is a screenshot of the error: http://fgasking.files.wordpress.com/2012/07/moodle-error.png

sammarshallou commented 12 years ago

This is not a general bug (i.e. I can type double spaces into ouwiki and it will still work just fine).

It is probably caused because when you enter multiple spaces, the TinyMCE editor turns them into non-breaking spaces which are outside the ASCII range. Most likely, other non-ASCII characters would also cause the same problem for you.

1) Are you sure that your database is correctly configured for UTF-8? 2) If you edit a wiki page and add the following Japanese text 日本語 does it cause the same problem? 3) If so, what happens if you put that Japanese text somewhere else in Moodle, such as a forum post?

tjwelde commented 11 years ago

So, I stumbled across this issue, while testing the master version (with our modifications) with moodle2.5

I found out, that the invalid character occurs only, if (even one) space is placed at the beginning, or at the end of a line

jason-platts commented 11 years ago

Timo, do you have a (small) example of the original and after preg_replace text?

tjwelde commented 11 years ago

sry, I was wrong. Looked at the wrong vars. I will digg down further and update in a few minutes

tjwelde commented 11 years ago

Ok, definetely found it: locallib.php:2067 it's a preg_replace, which should replace all whitespace into single spaces $content = preg_replace('/\s+/', ' ', $content);

jason-platts commented 11 years ago

Thanks, but I can't seem to replicate the situation you describe.

What text are you using and what text editor/format (I've tried switching from TinyMCE to a plain textarea and 'Plain text format' - but still leading spaces get trimmed)?

tjwelde commented 11 years ago

I am using the TinyMCE and am writing normal text, w/o puntuation or something else.

Hello World

where World has a space before and after it

tjwelde commented 11 years ago

maybe it is the php version? if i replace it with: '' (empty), the invalid character still gets introduced o.O

tjwelde commented 11 years ago

It works with the u modifier http://php.net/manual/en/reference.pcre.pattern.modifiers.php But still didn't find an issue for it on php.net... I am using php 5.3.6

jason-platts commented 11 years ago

Strange, TinyMCE usually wraps you text for you; so it would actually be

that was sent? Using this text I get no error and have a single space saved as the value in the database.

If I add two spaces I do find that they don't get converted into one - both spaces get saved to the database. So this might be a combination of ouwiki_format_xhtml_a_bit() not cleaning up all the text, plus MYSQL (either generally, or something in your DB setup?).

If I can get the text to clean up better I'll try and fix (thanks for u modifier tip), but I'd look into your database setup as presumably you could get this problem with other Moodle plugins.

FYI, I'm running: Moodle 2.5.1 Postgres PHP 5.4.19

tjwelde commented 11 years ago

I don't have this issue with other Moodle Plugins. Maybe Postgres takes it better, but I have the feeling, it has something to do with the php version resp. pcre version I am debugging with eclipse and can definitely say, that it is the preg_replace, which is introducing the invalid character. The u modifier isn't necessary. It would be enough to search for /\\s+/ instead of /\s+/ Sry, I am a bit tired today. Only u modifier solves it for me...

tjwelde commented 11 years ago

Works on php 5.3.15, pcre 8.12 and postgres with the issue, that double spaces don't get replaced Doesn't work on php 5.3.8, pcre 8.12 and postgres, with the same issue i have locally (php 5.3.6 & 5.4.10, pcre 8.31, mysql)

jason-platts commented 11 years ago

Thanks for the info - I've also found the whitespace stripping a bit hit and miss. The u modifier works though so I've added that into the code along with a fallback in case the utf8 validation that introduces fails.

This is in our MASTER branch and will be updated the next time we push our repos to GitHub (I'll try and get it done this week).

jason-platts commented 11 years ago

This fix is now in github

https://github.com/moodleou/moodle-mod_ouwiki/commit/d653eb42c7d7575ccf4530143ba3e76c50dbd497

Hopefully this works for you as it did for me...

tjwelde commented 11 years ago

Works perfectly, thanks!