Moodle

Forum Display Table Nesting Issue

Details

  • Type: Bug Bug
  • Status: Open Open
  • Priority: Trivial Trivial
  • Resolution: Unresolved
  • Affects Version/s: 1.5.3, 1.8.10, 1.9.6, 2.0
  • Fix Version/s: STABLE backlog
  • Component/s: Forum, General
  • Labels:
  • Environment:
    Linux
  • Database:
    Any
  • Affected Branches:
    MOODLE_15_STABLE, MOODLE_18_STABLE, MOODLE_19_STABLE, MOODLE_20_STABLE

Description

When displaying a list of forums (mod/forum/index.php...),

If a forum contains an HTML table and the forum is truncated by the parser before the </table> tag is reached, subsequent forums are displayed in a nested table format.

Activity

Ascending order - Click to sort in descending order
Hide
Permalink
Matthew Davidson added a comment - - edited

I've run against this many times because we use the forum as a "site news" thing...and we put pictures in it and we always want to be able to use tables.

I've been working on a project that ran into this same problem....this is how I fixed it. I ran this function after I truncated the posts. It might not be perfect, but it is definately better than the entire page being screwed up because of open table tags.

function closetags($html)
{
$selfclosing = ',img,input,br,hr,';

//put all opened tags into an array
preg_match_all("#<([a-z]+)(?: .*)?(?<![/|/ ])>#iU", $html, $result);
$openedtags=$result[1];

//put all closed tags into an array
preg_match_all("#</([a-z]+)>#iU",$html,$result);
$closedtags=$result[1];
$len_opened = count($openedtags);

//all tags are closed
if(count($closedtags) == $len_opened)

{ return $html; }

$openedtags = array_reverse($openedtags);

//close tags
for($i=0;$i < $len_opened;$i++)
{
$temp = $openedtags[$i];
switch ($openedtags[$i])
{
case strstr($selfclosing,",$temp,"):
break;
default:
if (!in_array($openedtags[$i],$closedtags))

{ $html .= '</'.$openedtags[$i].'>'; }

else

{ unset($closedtags[array_search($openedtags[$i],$closedtags)]); }

}
}
return $html;
}

Show
Matthew Davidson added a comment - - edited I've run against this many times because we use the forum as a "site news" thing...and we put pictures in it and we always want to be able to use tables. I've been working on a project that ran into this same problem....this is how I fixed it. I ran this function after I truncated the posts. It might not be perfect, but it is definately better than the entire page being screwed up because of open table tags. function closetags($html) { $selfclosing = ',img,input,br,hr,'; //put all opened tags into an array preg_match_all("#<([a-z]+)(?: .*)?(?<![/|/ ])>#iU", $html, $result); $openedtags=$result[1]; //put all closed tags into an array preg_match_all("#</([a-z]+)>#iU",$html,$result); $closedtags=$result[1]; $len_opened = count($openedtags); //all tags are closed if(count($closedtags) == $len_opened) { return $html; } $openedtags = array_reverse($openedtags); //close tags for($i=0;$i < $len_opened;$i++) { $temp = $openedtags[$i]; switch ($openedtags[$i]) { case strstr($selfclosing,",$temp,"): break; default: if (!in_array($openedtags[$i],$closedtags)) { $html .= '</'.$openedtags[$i].'>'; } else { unset($closedtags[array_search($openedtags[$i],$closedtags)]); } } } return $html; }
Hide
Permalink
Jonathan Harker added a comment - - edited

If you are running PHP5 and have the php-tidy extension installed, this bug can also fixed in the forum_shorten_post() function in mod/forum/lib.php:

-   return substr($message, 0, $truncate);
+   $truncatedpost = substr($message, 0, $truncate);
+   if (function_exists('tidy_repair_string')) { // requires php-tidy extension
+       return tidy_repair_string($truncatedpost);
+   } else {
+       return $truncatedpost;
+   }
Show
Jonathan Harker added a comment - - edited If you are running PHP5 and have the php-tidy extension installed, this bug can also fixed in the forum_shorten_post() function in mod/forum/lib.php: 
-   return substr($message, 0, $truncate);
+   $truncatedpost = substr($message, 0, $truncate);
+   if (function_exists('tidy_repair_string')) { // requires php-tidy extension
+       return tidy_repair_string($truncatedpost);
+   } else {
+       return $truncatedpost;
+   }
Hide
Permalink
Jonathan Harker added a comment -

Applies cleanly to CVS HEAD (2.0), MOODLE_19_STABLE, and MOODLE_18_STABLE.

Show
Jonathan Harker added a comment - Applies cleanly to CVS HEAD (2.0), MOODLE_19_STABLE, and MOODLE_18_STABLE.
Hide
Permalink
Jonathan Harker added a comment -

Fix relies on the php-tidy extension, falls back to normal (broken) behaviour if the extension is absent. This is a silent dependence on php-tidy, so I'm not sure whether to add it to admin/environment.xml as an optional module, or fix it permanently by making it required (I imagine on HEAD only as we probably don't want to clobber existing 1.8 and 1.9 hosting environments).

Comments on the above please - I'd like to commit this soon

Cheers, J

Show
Jonathan Harker added a comment - Fix relies on the php-tidy extension, falls back to normal (broken) behaviour if the extension is absent. This is a silent dependence on php-tidy, so I'm not sure whether to add it to admin/environment.xml as an optional module, or fix it permanently by making it required (I imagine on HEAD only as we probably don't want to clobber existing 1.8 and 1.9 hosting environments). Comments on the above please - I'd like to commit this soon Cheers, J
Hide
Permalink
Martin Dougiamas added a comment -

I think putting it in environment checks as optional is a good idea, and keep the code exactly as you have it. Great, thanks!

Show
Martin Dougiamas added a comment - I think putting it in environment checks as optional is a good idea, and keep the code exactly as you have it. Great, thanks!
Hide
Permalink
Sam Hemelryk added a comment -

Hi Jonathan,

Just an idea but maybe we should think about creating a truncate function that respects HTML tags and does some other nice stuff at the same time.
This would allow us to use the function in other places and doesn't rely on HTML tidy.

I've had think about how this function would, or could operate and came up with the solution in the attached truncate.php file.
I am aware of potentially a couple of issues, such that the function relies on properly nested HTML (don't know what the output would be like with bad html) and expects the first > after a < to be the closing of a tag... really it just expects chars to be encoded properly.
Have a read of the comments for the function and let me know what you think

Cheers
Sam

Show
Sam Hemelryk added a comment - Hi Jonathan, Just an idea but maybe we should think about creating a truncate function that respects HTML tags and does some other nice stuff at the same time. This would allow us to use the function in other places and doesn't rely on HTML tidy. I've had think about how this function would, or could operate and came up with the solution in the attached truncate.php file. I am aware of potentially a couple of issues, such that the function relies on properly nested HTML (don't know what the output would be like with bad html) and expects the first > after a < to be the closing of a tag... really it just expects chars to be encoded properly. Have a read of the comments for the function and let me know what you think Cheers Sam
Hide
Permalink
Martin Dougiamas added a comment -

Looks good Sam. Perhaps we could use a function like this when HTML tidy isn't present, Jonathan?

Show
Martin Dougiamas added a comment - Looks good Sam. Perhaps we could use a function like this when HTML tidy isn't present, Jonathan?
Hide
Permalink
Jonathan Harker added a comment -

Ah, true enough In that case, do we want to just add sam's code to 1.8 and 1.9, and use it in the fallback? In 2.0 we could get away with making php-tidy required.

Show
Jonathan Harker added a comment - Ah, true enough In that case, do we want to just add sam's code to 1.8 and 1.9, and use it in the fallback? In 2.0 we could get away with making php-tidy required.
Hide
Permalink
Jonathan Harker added a comment -

Sorry I've been swamped - I'll try to get on to committing it this week.

Show
Jonathan Harker added a comment - Sorry I've been swamped - I'll try to get on to committing it this week.
Hide
Permalink
Dean Lewis added a comment -

In order to make Jonathan's patch work correctly you also need to add extra parameters to the tidy_repair_string function call. So it should be - return tidy_repair_string($truncatedpost, NULL, latin1) - adding this will stop strange characters displaying infront of £ and " symbols.

Show
Dean Lewis added a comment - In order to make Jonathan's patch work correctly you also need to add extra parameters to the tidy_repair_string function call. So it should be - return tidy_repair_string($truncatedpost, NULL, latin1) - adding this will stop strange characters displaying infront of £ and " symbols.

People

Vote (4)
Watch (2)

Dates

  • Created:
    Updated:
Atlassian JIRA (v4.4.3#663-r165197) | Report a problem
Powered by a free Atlassian JIRA open source license for Moodle. Try JIRA - bug tracking software for your team.