[RFC] Integrate s9e\TextFormatter

[JoshyPHP]

I hate seeing unrendered tags in posts

I too, would love to omit bad tags from the output, but I haven't found a way that works reliably. I believe it's a problem that does not have a definitive solution. I've sunk a lot of time thinking about it while developing the library, and we could collectively sink a lot more without any actual progress. The only way to definitively eliminate bad markup is to not use markup. Unfortunately, rich text editors come with their own set of challenges and shortcomings. So we're stuck with imperfect solutions such as the ones we're discussing.

Worse,if he's too lazy to Preview his post, he's probably not going to look at it after he posts

In this case, what will that user do when presented with an error message? Go back and fix the text, click the button that says "I don't care, post anyway" or give up on posting the message altogether?

I'd say we go with option 1 steve chose and just insert a bold tag directly prior to the quote tag. IMO that is the best option in such a case.

As a reminder, the default behaviour in s9e\TextFormatter would be to disable the quote tag in that context. Currently, the one way to achieve what you describe would be to manually add a rule that says "quote tags close bold tags" and that would need to be replicated for every tag that you want closed automatically. At some point in the future, it should be possible to add only one global rule that says "all block tags should close non-block tags" and I'd love to get ideas about that. Not just about phpBB, but in general. Because even though "all block tags should close non-block tags" sounds good, it doesn't mean anything in HTML5 because there's no such thing as a block tag. The concept of block elements vs inline elements dates from HTML4 and found its way in CSS as the "display" attribute but even in CSS it's not that clear. For instance, tables are not blocks, they're "display: table".

EDIT: We could just do what we do now with the unclosed tag in the first paragraph of this post: nothing. Don't parse it. Leave it be.

The default behaviour in s9e\TextFormatter is to open tags even if they don't have an end tag immediately apparent. The main reason is that not every tag has to be explicitely closed, such as [*] or in some other commenting systems [hr] and other similar tags. This can be changed on a per-BBCode basis using an option named forceLookahead, which forces the BBCodes parser to look ahead in the text for a potential closing tag. Unlike most rules, s9e\TextFormatter doesn't have an automated way to set this option based on templates.

What about cases where they try to quote inside of bold though? Shouldn't it be smart enough to close the bold and then restart the after the close quote?

Currently that's not possible, but that's definitely something I'll consider adding to s9e\TextFormatter even if it's not used in phpBB. I'd have to think about it because I'm afraid of bad interactions between tags being automatically closed and tags being automatically reopened; I wouldn't want to create a repeating loop, even though there are safeguards in place that would limit it to "only" a few hundreds extraneous tags rather than a CPU-melting, memory-hogging, infinite loop.

PS: if you want to test the default behaviours, you can try this JavaScript demo. It's not the same BBCodes as phpBB's, but it's real-time and it gives you an idea.

[JoshyPHP]

I'm almost done with the TODO list that's on the first page. The last few remaining items are:

Tabs in code blocks are preserved, legacy routines replaces them.

Haven't looked into it yet. I'm probably going to post-process the output to replace tabs with the unholy mixture of spaces and   that the legacy routines use.

Cron job that removes old renderers from the cache

I've added a function to remove old cache files, callable as $phpbb_container->get('text_formatter.cache')->tidy(). But instead of making it a cron job, I just remove all compiled renderers when "Purge the cache" is used in the ACP. I've actually crossed that one will typing this.

Look into how text is cleaned up for the search engine

I've already looked into it. Only the "native" search backend cleans up the text, and what it does (strip tags, remove BBCodes) works the same way whether the text is legacy or new, so I don't think I have to touch it. Long-term, I'd recommend moving the cleanup method out of the native search backend so it can be reused by other backends. (Sphinx maybe?) The text_formatter.utils service has methods that can help.

That path_in_domain() thing

I kept this note for myself, to remind me to look into path_in_domain() and figure out what it does. I've come to the conclusion that it has no effect and does not have to be ported. I can only speculate about its original purpose, but its implementation is broken and has been for a little more than 6 years. In short, the function can only ever return false so it's effectively been a no-op for years.

[Master_Cylinder]

Again, I'm no programmer but I'm not sure why it would be impossible or bad if it sees this:

Code: Select all

[b]some text blah blah [quote]something quoted[/quote] more text blah blah[/b]

Then I would think that it should be able to convert it to:

Code: Select all

[b]some text blah blah[/b][quote]something quoted[/quote][b] more text blah blah[/b]

Without causing loops or other bad behavior...but you're the expert not me...

*shrug*

[JoshyPHP]

I'm thinking about loops that could happen in some other cases. I didn't specify, sorry.

It might be theorically possible to create a set of rules that would cause two tags to automatically close each other, and automatically reopen them. When used one after the other, they would indefinitely open/close each other. In fact, they would be limited to a few hundreds iterations (which would only take a millisecond) so it wouldn't be infinite and it wouldn't hurt the server, but I try to be very thorough when it comes to unbounded loops.

[Master_Cylinder]

Some of us that don't program often underestimate the complexity of some things that seem simple. I've worked with a lot of programmers but I don't write much myself so I throw stuff out there and see if it sticks...

[keith10456]

Really exciting stuff! Thanks Joshy for the contribution!

[keith10456]

Just a note in reference to issue of displaying an error message or not... Most users just want it work. Error messages, etc. is just noise that's interfering with them accomplishing their goal - to post the message. That said, if the corrections can be handled on the back-end, then so be it.

Currently, if bbcode is used incorrectly an error message isn't displayed. We're simply showed the messed-up post - and if what you see is not what you intended you go back and fix the problem.

So there's no reason to show an error message if the problem is auto-corrected (ie auto-spell check). If what you see is not what you intended, you go back and fix the problem. At least with the auto-correct you're less likely to have to edit your post again.

[Pony99CA]

Currently, if bbcode is used incorrectly an error message isn't displayed. We're simply showed the messed-up post - and if what you see is not what you intended you go back and fix the problem.

You're presuming that people have the ability to edit their posts (or would even notice the problem after they post). If they get an error message and their post doesn't show up in the topic, that's pretty hard to ignore.

But I'm a programmer at heart and hate bad code, so I hate posts with botched BBCode. Other admins may differ, hence my suggestion of having an option to prevent illegal BBCode. As I alluded to, that option may be turned off at phpbb.com but turned on here at Area51 (we should all know how to properly format BBCode here, right?).

Steve

[Un1matr1x]

You're presuming that people have the ability to edit their posts (or would even notice the problem after they post). If they get an error message and their post doesn't show up in the topic, that's pretty hard to ignore.

Bugreport: I hit the [Submit]-Button and no post is shown now, what went wrong?
Would you think of broken-bb-code-error-msg was ignored in the first place? I know too many people that just hit the button that might be the right, close their eyes & hope for the best.

PS: Just 4 [b]Ponny99CA[/u][/b]]

[Pony99CA]

You're presuming that people have the ability to edit their posts (or would even notice the problem after they post). If they get an error message and their post doesn't show up in the topic, that's pretty hard to ignore.

Bugreport: I hit the [Submit]-Button and no post is shown now, what went wrong?
Would you think of broken-bb-code-error-msg was ignored in the first place? I know too many people that just hit the button that might be the right, close their eyes & hope for the best.

It would be hard to imagine that they didn't notice they were still in Preview mode. Yes, some doorknobs might open a bug, but it's not like that doesn't already happen today.

We already give error messages for things like images being too large and stick in Preview mode if somebody else has posted and the Enable post review forum option is on. This would be no different.

And remember, I'm not trying to jam it down your throat; I'm asking for the option to allow blocking malformed BBCode posts. Putting the admin in control of his own forum is a good thing.

Steve