Description

The diff on the history page is not always working (= doesn't show the added content or doesn't show anything at all anymore)
I could reproduce this by pasting some content from a Word document in the wiki.
Since teachers find it important to be able to know what each student add, I've set the priority to major.
Tested on 1.6.3 and 1.7
Also found this forum discussion describing the same problem: http://moodle.org/mod/forum/discuss.php?d=54529

Gliffy Diagrams

Activity

<P>This appears to effect all versions, at least 1.5.3+ to 1.8
<P>I believe I've found the fix for the problem.
<P>In the file:<BR>
moodle/mod/wiki/ewiki/plugins/moodle/diff.php
<P>Line 17 and 18 (just below this label):
/// Replace <p>&nbsp;</p>
<P>The regexes are:<BR>
<STRONG>(<p.<EM>>(&nbsp;|\s+)</p>|<p.</EM>></p>)</STRONG>i
<P>That is bad, they should be:<BR>
<STRONG>(<p.<EM>>(&nbsp;|\s+)</p>|<p.</EM>^>></p>)</STRONG>i
<P>Basically the part:<BR>
<p.*></p> will also match <p><b>test</b></p> because the "><b>test</b" counts as *, so you have <p"><b>test</b"></p> which it thinks is an empty <p> block.
<P>The ^> says that there cant be any >'s in the *, which eliminates the problem

Eric Merrill
added a comment - 24/Mar/07 4:59 AM <P>This appears to effect all versions, at least 1.5.3+ to 1.8
<P>I believe I've found the fix for the problem.
<P>In the file:<BR>
moodle/mod/wiki/ewiki/plugins/moodle/diff.php
<P>Line 17 and 18 (just below this label):
/// Replace <p>&nbsp;</p>
<P>The regexes are:<BR>
<STRONG>(<p.<EM>>(&nbsp;|\s+)</p>|<p.</EM>></p>)</STRONG>i
<P>That is bad, they should be:<BR>
<STRONG>(<p.<EM>>(&nbsp;|\s+)</p>|<p.</EM>^>></p>)</STRONG>i
<P>Basically the part:<BR>
<p.*></p> will also match <p><b>test</b></p> because the "><b>test</b" counts as *, so you have <p"><b>test</b"></p> which it thinks is an empty <p> block.
<P>The ^> says that there cant be any >'s in the *, which eliminates the problem

Eric Merrill
added a comment - 24/Mar/07 4:59 AM This appears to effect all versions, at least 1.5.3+ to 1.8
I believe I've found the fix for the problem.
In the file:
moodle/mod/wiki/ewiki/plugins/moodle/diff.php
Line 17 and 18 (just below this label):
/// Replace <p> </p>
The regexes are:
#(<p. >( |\s+)</p>|<p. ></p>)#i
That is bad, they should be:
#(<p. >( |\s+)</p>|<p. ^>></p>)#i
Basically the part:
<p.*></p> will also match <p><b>test</b></p> because the "><b>test</b" counts as *, so you have <p"><b>test</b"></p> which it thinks is an empty <p> block.
The ^> says that there cant be any >'s in the *, which eliminates the problem

Unfortunately I didn't look for this bug until after already tracking down the issue on my own, would have saved me some time as the problems identified here were the correct ones.

Anyway I have fixed this and applied the patch to 1.8 branch and HEAD.

I used the regexp:

<p( [^>]*)?>

In other words, <p followed optionally by space and some other non-> characters, then >. The space ensures that this won't match other HTML tags beginning with <p> (<param> and <pre> are the only ones, but still). Now that I think of it, it should really be \s not space, but nobody would in practice use other character there, so it's probably ok Anyway it seems to fix the problem at least in my example case.

Sam Marshall
added a comment - 08/Jun/07 9:51 PM Unfortunately I didn't look for this bug until after already tracking down the issue on my own, would have saved me some time as the problems identified here were the correct ones.
Anyway I have fixed this and applied the patch to 1.8 branch and HEAD.
I used the regexp:
<p( [^>] *)?>
In other words, <p followed optionally by space and some other non-> characters, then >. The space ensures that this won't match other HTML tags beginning with <p> (<param> and <pre> are the only ones, but still). Now that I think of it, it should really be \s not space, but nobody would in practice use other character there, so it's probably ok Anyway it seems to fix the problem at least in my example case.

Eric Merrill
added a comment - 08/Jun/07 9:52 PM "Actually the lines should be changed to the following:"
Yeah, I was just tinkering and came up with my version. After actually thinking about it, yours is definitely the 'correct' way to do it (hey, mine worked too in my testing )