[RFC] Integrate s9e\TextFormatter

These requests for comments/change have lead to an implemented feature that has been successfully merged into the 3.2/Rhea branch. Everything listed in this forum will be available in phpBB 3.2.
Post Reply
User avatar
nickvergessen
Former Team Member
Posts: 733
Joined: Sun Oct 07, 2007 11:54 am
Location: Stuttgart, Germany
Contact:

Re: [RFC] Integrate s9e\TextFormatter

Post by nickvergessen » Mon Feb 16, 2015 10:36 pm

Well adding a url field to the author on quotes is only half the feature we had before.
Previously you could also color it (e.g. with the administrator color on admin posts)
[color=#FF0000]ADMIN[/color] wrote:sample
With all these BC breaks, I personally would not like to have it changed. However I can also see the PR growing infinite, with all the requests and features we have and want to have, so I'm really split here.
Member of the Development-TeamNo Support via PM

User avatar
brunoais
Registered User
Posts: 958
Joined: Fri Dec 18, 2009 3:55 pm

Re: [RFC] Integrate s9e\TextFormatter

Post by brunoais » Mon Feb 16, 2015 10:58 pm

Although that is possible... I really believe it is really too rarely used to make sense to give support to it.
Also, was that in the use-cases of it or was that just something that happened of how the BBCode parser works? I'd bet on the latter based on the comments and how the code is written.
(not exactly, but part of it: https://github.com/phpbb/phpbb/blob/dev ... er.php#L88 ).
Also, there was no information (wen the parser was made) that that use-case had been taken into account.

Assuming the above is correct, making that not happening anymore isn't changing the use-cases of the tag. The new use-cases that later had appeared due to the way it parses become moot given that the major use-cases for it (a link to the original) becomes available.

User avatar
JoshyPHP
Registered User
Posts: 350
Joined: Fri Jul 08, 2011 9:43 pm

Re: [RFC] Integrate s9e\TextFormatter

Post by JoshyPHP » Tue Feb 17, 2015 5:24 am

nickvergessen wrote:Well adding a url field to the author on quotes is only half the feature we had before.
If the argument is 1 BBCode = 1 feature then it's only 16.67% of what you had before (b, i, u, color, url and email are all allowed in the quote's author field.) And it's even less when you consider that in the current system you can use multiple URLs and multiple colors and multiple underlines (etc...) in the quote's author.

Realistically, apart from the url BBCode I don't think those features will be missed by many people. Or by anybody really. For those who do miss them, it will be possible to work something out as an extension as soon as we create events to give them access to the library. And even without an extension, if someone really wants rich quote authors they can create a custom BBCode that's used for the quote's author line.

I'm going to add the URL-enhanced quote BBCode tomorrow. It will automatically extract the first URL it finds in the quote's author. That means that the following will be functionally equivalent and they won't require a migration.

Code: Select all

[quote="[url=http://example.org]author[/url]"]...[/quote]
[quote="author" url="http://example.org"]...[/quote]
naderman wrote:Censoring domains or URLs is a very common practice so we need a solution, either a new feature to blacklist URLs or a modification of how the censors work.
I have spent time on this and I've come to the conclusion that there is no simple, clean way to have a censor that is:
  • fully dynamic
  • HTML-safe
  • works on URLs
You can get two of them without too much work, but to get all three requires either a lot of work or it will be a hack.

You can get a censor that is fully dynamic on the post's text, HTML-safe and blocks URLs at parsing time. The URL blocking would not be retroactive.

Or you can just run censor_text() on the HTML. It's not HTML-safe but that's closer to what 3.1 does.

There are many possibilities but all of them involve a trade-off. To keep things in perspective, regardless of how thorough the link censor is, all it takes to bypass it is a URL shortener.

Edit: after some more brainstorming, I think I can use a modified version of the HTML-safe censor I use in the library to censor text nodes and the content of HTML attributes. That might be the closest thing to how 3.1 behaves. I'll look into it ~tomorrow.

User avatar
JoshyPHP
Registered User
Posts: 350
Joined: Fri Jul 08, 2011 9:43 pm

Re: [RFC] Integrate s9e\TextFormatter

Post by JoshyPHP » Mon Feb 23, 2015 6:14 pm

Status update
Links point to the relevant commit, commits usually contain regression tests.

Added limited support for url in quote author.
The following forms are supported:

Code: Select all

[quote="[url=http://example.org]author[/url]"]...[/quote]
[quote="[url]http://example.org[/url]"]...[/quote]
[quote="http://example.org"]...[/quote]
[quote="author" url="http://example.org"]...[/quote]
Allowed text in places where text is not valid HTML.
That's consistent with 3.1.

Replaced the Censor plugin with something that is run at rendering time.
The behaviour is close to 3.1's, with lower overhead. The censor applies to plain text and attribute values. Does not apply to tag names or attribute names.

Outstanding issues
  1. Following up:
    naderman wrote:
    JoshyPHP wrote:
    • Default limits are set on the number of occurence of individual BBCodes in text (1000 of each), global number of BBCodes in text (10000) and nesting level (10.) Same thing for smilies, the default is 1000 smilies per post. Past this limit, the markup is ignored.
    Why?
    If you want to change those limits, I need a number between 1 and PHP_INT_MAX.
  2. I need to create at least two events that trigger before and after the library is configured. That would allow extensions to modify almost any aspect of text formatting. I'm not sure how you'll want them named. I guess core.text_formatter_s9e_configure_before and core.text_formatter_s9e_configure_after?
Next
Nicofuma wrote:I would like to finish that as soon as possible, but I also see that there is still an open discussion with no agreement nor consensus about the storage, and I think that this point is crucial and need to be decided.
With the storage question seemingly settled, what's the next step?

User avatar
brunoais
Registered User
Posts: 958
Joined: Fri Dec 18, 2009 3:55 pm

Re: [RFC] Integrate s9e\TextFormatter

Post by brunoais » Mon Feb 23, 2015 10:49 pm

JoshyPHP wrote: [*]I need to create at least two events that trigger before and after the library is configured. That would allow extensions to modify almost any aspect of text formatting. I'm not sure how you'll want them named. I guess core.text_formatter_s9e_configure_before and core.text_formatter_s9e_configure_after?
My opinion is (with what I know):
core.bbcode_parsing_before
core.bbcode_parsing_after

Depending on the code structure, I may find better names. Regardless, I would never use the names you suggested :).

User avatar
JoshyPHP
Registered User
Posts: 350
Joined: Fri Jul 08, 2011 9:43 pm

Re: [RFC] Integrate s9e\TextFormatter

Post by JoshyPHP » Mon Feb 23, 2015 10:51 pm

  • The service is called text_formatter, it's not specific to BBCodes
  • The specific implementation is s9e (other implementations are theorically possible)
  • The event is triggered when the library is configured, not during parsing
Extensions that want to access the library before/after parsing/rendering can do it through the service, via the core.modify_text_for_* events. For example:

Code: Select all

$parser_service = $phpbb_container->get('text_formatter.parser');
if ($parser_service instanceof \phpbb\textformatter\s9e\parser)
{
    // Returns an instance of s9e\TextFormatter\Parser
    $parser_service->get_parser();
}

User avatar
brunoais
Registered User
Posts: 958
Joined: Fri Dec 18, 2009 3:55 pm

Re: [RFC] Integrate s9e\TextFormatter

Post by brunoais » Mon Feb 23, 2015 11:15 pm

Ah, so the event is meant to be specific for your BBCode parser, is that it?

User avatar
JoshyPHP
Registered User
Posts: 350
Joined: Fri Jul 08, 2011 9:43 pm

Re: [RFC] Integrate s9e\TextFormatter

Post by JoshyPHP » Mon Feb 23, 2015 11:23 pm

The event is for accessing the s9e\TextFormatter library before/after it's configured with phpBB's default configuration. It will let extensions modify BBCodes, smilies, the word censor, etc...

User avatar
brunoais
Registered User
Posts: 958
Joined: Fri Dec 18, 2009 3:55 pm

Re: [RFC] Integrate s9e\TextFormatter

Post by brunoais » Tue Feb 24, 2015 8:17 am

I think there should be something before that, then. Something that allows to alter by the phpBB level and not already inside textFormatter itself.
The event should allow altering all those parameters but it should use a general purpose interface that should be equal to every BBCode parser that phpBB may have (from the current plans)

User avatar
JoshyPHP
Registered User
Posts: 350
Joined: Fri Jul 08, 2011 9:43 pm

Re: [RFC] Integrate s9e\TextFormatter

Post by JoshyPHP » Tue Feb 24, 2015 8:38 am

I'm not sure what you're suggesting exactly. This event gives access to an instance of s9e\TextFormatter\Configurator in order to use features that are specific to this library. That's why it cannot be a generic event.

Also, it is important to not confuse BBCode parsers with this set of services. This PR is not limited to BBCodes. Smilies are not BBCodes, neither is the word censor. In phpBB, magic links are not BBCodes either. That's why the services are called text_formatter.* and not something specific such as bbcodes.*

Post Reply