PHPBB3-13803 / PHPBB3-13891 - Reparsing text

[JoshyPHP]

I wouldn't recommend setting it to the plugin's name. As far as I know, only 2 values are ever used for that parameter outside of the reparser: "post" and "sig". Keeping it that way would be the safe option.

[rxu]

Well yes, but reparser handles more places (posts, PMs, polls, forum rules, group descriptions, signatures), so having just 2 options is kind of not enough.

[JoshyPHP]

If you use the plugin's name as mode, be aware than the plugin that reparses users signatures is named user_signature whereas the mode used when editing a signature the conventional way is sig. Same for post_text and post.

[rxu]

What about that (within the reparser's base.php)

Code: Select all

	/**
	* Gets the current reparser plugin class name
	*/
	public function get_name()
	{
		$reflection = new \ReflectionClass($this);
		return $reflection->getShortName();
	}

and then

Code: Select all

		generate_text_for_storage(
			$text,
			...
			$unparsed['enable_url_bbcode'],
			'reparse.' . $this->get_name()
		);

[JoshyPHP]

The issue is the same: I don't see any benefit to introducing new modes. As far as I'm aware there are only 3 values ever used for $mode in phpBB 3.2: "post", "sig" and since 3.2, "reparse". I think it's safer to ditch "reparse" and just keep the two known ones.

[MattF]

I think that the correct way to do it would be a big overhaul of message_parser and the generate_text_for_* functions into a unified API that supports some sort of contextual configuration that would allow the text to be treated differently depending on whether it's from a post, a signature, etc...

For a more immediate solution, reparser_interface could be extended to some sort of get_parsing_mode() function, the return value of which would be passed to generate_text_for_storage(). Currently, it is hardcoded to "reparse", which, in hindsight, is not a good value. phpbb\textreparser\base::get_parsing_mode() could return "post" by default and the user_signature plugin would return the appropriate value instead. As I recall, that value is already present in the relevant events.

I made a PR here to set names for reparsers:
https://github.com/phpbb/phpbb/pull/4585

I'm not so sure setting 'reparse' to 'post' is a good idea for the reparsers by default. Because when you use post, it enforces settings like max/min characters allowed. If users have changed that setting over the years, reparsing could fail on posts that are too big or too small. In fact it seems the post mode is only used to enforce max/min characters, smilies, URLs, etc.

In the PR, the "short" name of the reparser is used instead of just 'reparse'. Like 'reparse' it's just not used for anything other than to allow the reparsers to succeed without worrying about posting limits, but by using the reparsers name it is a bit specific which is nice if the core or an extension wants to target a specific reparser action.