Updated Inline Thread Popup Review

[ZiNgA BuRgA]

This is a plugin "review" requested by FBI.


This plugin doesn't seem to have huge adverse effects, however I do have quite a number of concerns.

My biggest concern is the following piece of code:

PHP Code:

1 2 3 4 5 6 7 8

$message = $parser->parse_message($post['message'], $parser_options); if(strlen($message) > $mybb->settings['pcssp_message']) { $message = substr($message, 0, $mybb->settings['pcssp_message']); $message .= "..."; } $message = preg_replace('|<(.*?)\.\.\.|', '', $message);

---

To explain the logic, it basically takes the text that the user enters for the first post, then sends it to the parser to convert bbCode into HTML.  After the conversion, it will take this HTML and chop it at the maximum length specified.
To the observant reader, this methodology has a number of problems:

• HTML tags are considered in the length
• It's possible for this to chop off tags in the middle

For the last point, the last line of code seems to try to mitigate the issue.  I'm guessing it's trying to remove open tags at the end.  Unfortunately, the regex isn't accurate and will remove everything after any tag.
For example, set the maximum length to 10, and make a new thread with the following post:

Code:

abc[b]def[/b]

---

The above will generate a preview of "abc".
The placement of preg_replace is also susceptible to triple dots in posts, eg, even if we set a maximum of 200 chars, the following post will break previews:

Code:

I [b]don't[/b] know... What should I do?

---

Fixing the above correctly isn't terribly easy - it would require a HTML parser which properly counts length.  Doing this may cause additional slowdown to the board, however this can actually be mitigated through a preparser.

Which brings to the next issue, the parser is called for every thread on forumdisplay, which can increase load a bit, considering that forumdisplay is usually an often visited page.  Preparsing would solve this issue, although, arguably, MyBB itself doesn't include a post preparser.
I do like the use of caching first posts - something many plugin authors seem to forget to do.

Another concern I have is that template edits aren't performed during activation (FBI comments that he removed it) however it's still being done in deactivation.  I guess it works, but it seems a bit odd.

Lastly, this does leave various tidbits from the MyBB 1.2.x era, such as:

PHP Code:

1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30

if(!function_exists("rebuildsettings")) { function rebuildsettings() { global $db; if(!file_exists(MYBB_ROOT."inc/settings.php")) { $mode = "x"; } else { $mode = "w"; } $options = array( "order_by" => "title", "order_dir" => "ASC" ); $query = $db->simple_select("settings", "value, name", "", $options); while($setting = $db->fetch_array($query)) { $setting['value'] = str_replace("\"", "\\\"", $setting['value']); $settings .= "\$settings['".$setting['name']."'] = \"".$setting['value']."\";\n"; $mybb->settings[$setting['name']] = $setting['value']; } $settings = "<"."?php\n/*********************************\ \n DO NOT EDIT THIS FILE, PLEASE USE\n THE SETTINGS EDITOR\n\*********************************/\n\n$settings\n?".">"; $file = @fopen(MYBB_ROOT."inc/settings.php", $mode); @fwrite($file, $settings); @fclose($file); } }

---

The above code should be removed, and "rebuildsettings" should be replaced with "rebuild_settings"

PHP Code:

($post['smilieoff'] == "yes") ? "no" : $forum['allowsmilies'],

---

PHP Code:

'filter_badwords' => 'yes',

---

...etc
From memory, yes/no was converted to 1/0 in MyBB 1.4 (need to double check that)

PHP Code:

$db->query("DELETE FROM ".TABLE_PREFIX."settinggroups WHERE gid='".$gid."'"); $db->query("DELETE FROM ".TABLE_PREFIX."settings WHERE gid='".$gid."'");

---

Should use $db->delete_query for those.


Otherwise, it seems to be a good effort from FBI, who claims to be not a PHP coder