[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
Pony99CA
Registered User
Posts: 986
Joined: Sun Feb 08, 2009 2:35 am
Location: Hollister, CA
Contact:

Re: [RFC] Integrate s9e\TextFormatter

Post by Pony99CA »

I think that EX's point may be along the lines I'm thinking, too. I'd rather see an error message telling me that my tags aren't nested properly than try to have the system figure it out.

Yes, we have to support unclosed list items, because that's what phpBB currently does, but we shouldn't try to render crap BBCodes. If you insist on auto-correction, at least throw a warning message and preview the corrected post if they try to Submit bad code. That way the user will be told the code was bad, shown what the system thinks it should be and have a chance to correct it.

Steve
Silicon Valley Pocket PC (http://www.svpocketpc.com)
Creator of manage_bots and spoof_user (ask me)
Need hosting for a small forum with full cPanel & MySQL access? Contact me or PM me.

User avatar
EXreaction
Registered User
Posts: 1555
Joined: Sat Sep 10, 2005 2:15 am

Re: [RFC] Integrate s9e\TextFormatter

Post by EXreaction »

That, and also that this logic would be needed somehow for custom bbcodes, which isn't going to be easy for anyone to configure I wouldn't think. All these bbcodes must be movable to custom logic stored in the database.

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

Re: [RFC] Integrate s9e\TextFormatter

Post by JoshyPHP »

There are multiple, different arguments here so I'll try to separate them.

First, error messages. Error messages is what programmers want to read. Users, not so much. The problem with error messages is that they immediately break the flow of operation. The user wanted to post a message, but instead now they have to stop, read an error message, understand it, and then act on it otherwise their message is irremediably lost. The middle point is important, too, because some users don't even know what a "BBCode" is. Some users just click on buttons (e.g. the quote button, the B button) and hope it'll go well. And naturally, the people most likely to produce those error messages are the least likely to understand them.

Going back to the b/quote ticket, what would an error message read in this situation? In this instance, s9e\TextFormatter logs a warning (which is not picked up by phpBB) that reads "Tag is not allowed in this context". The log entry contains the name of the tag and the tag object itself, which contains the position and length of the tag, as well as other attributes. This is useful for a programmer, but it might be the least user-friendly warning possible. What would a user-friendly error message read? "Cannot use [quοte] inside of [b]"? Or "You must close [b] before opening [quοte]" or "End the bold text before opening a quote" maybe, to which the average user would reply "Why don't you do it since you know exactly where it is?" Because it's also relatively hard for the user to locate the portion of text that caused the error. You can't say "...at position 627" because it doesn't mean anything to a human, and even "line 12 column 3" is a hassle to locate when you don't have line numbers and columns in a textarea.

That's why error messages should generally be avoided when the user's intentions are clear enough. As long as the content is in a good enough shape it should be posted, and if you want to be thorough you can slap a warning on top of it. Something to point out to power users, "there might be a problem around *there*", similar to what Pony99CA proposes except that I'd rather do that without blocking the message. But all of that is a user interface issue though, and outside the scope of this PR. If this is targeting a 3.2 release, it means it's going to sit unmerged for an untold period of time (considering there isn't even a 3.2 branch yet) and I'd rather minize the risks of conflicts, comes merge time.
Pony99CA wrote:Yes, we have to support unclosed list items, because that's what phpBB currently does, but we shouldn't try to render crap BBCodes.
I believe it's just a question of sensitivites. For instance, for me who thinks in XML, <li><li> is an abomination, heresy! A li child of a li, unthinkable! But for most people, it makes perfect sense because the intentions are clear: that's two list items. I could draw a parallel to <b><i></b></i>. The intentions are pretty clear there too: things between "b" should be bold, things between "i" should be italic. The fact that it has to be expressed as <b><i></i></b><i></i> is the consequence of an imperfect system. (although the HTML5 specs explictly define how the former must be interpreted as the latter) I agree that crap (or rather, unsalvageable) text should generally produce an error and let the user correct it, we only disagree on what constitutes "crap."
EXreaction wrote:That, and also that this logic would be needed somehow for custom bbcodes
You'd have to specify what "this logic" refers to. I hope I've made it clear that all those rules already exist in s9e\TextFormatter, and that most of them are generated automatically. The only things here that are specific to phpBB is whether b/i/u should be automatically reopened as in the example above (and that's only because the default b/i/u templates use span elements rather than the more semantic elements found in bbcode.html) and whether a block-level element such as "quote" should automatically close a b/i/u, rather than be ignored in this context, which is the default action.
EXreaction wrote:which isn't going to be easy for anyone to configure I wouldn't think.
I agree that configuring tag rules is mind numbing. You'd go mental, configuring and allow/deny rule to define where each and every tag can be used (an exponential task), defining optional end tags, or how whitespace and new lines should be treated around each BBCode (otherwise you get a bunch of <br/> around quotes, lists, list items, etc...) That's why I find it damn convenient to be able to automatically generate those rules based on templates.

I'm considering making this "rules generator" more customizable but I haven't gotten to it yet, in part because of the time spent on this PR.
EXreaction wrote:All these bbcodes must be movable to custom logic stored in the database.
I'm not sure what "custom logic" would refer to in this context. If you want to store every BBCode in the database, you'd have to store some metadata in addition to the fields that are exposed in the current admin panel, otherwise you wouldn't be able to define that the content of a [code] tag should be free of URLs, smilies and BBCodes.

User avatar
EXreaction
Registered User
Posts: 1555
Joined: Sat Sep 10, 2005 2:15 am

Re: [RFC] Integrate s9e\TextFormatter

Post by EXreaction »

By logic I meant things like "whether a block-level element such as "quote" should automatically close a b/i/u, rather than be ignored in this context, which is the default action". If there is an easy way the custom bbcode editor can be built so this sort of logic is intuitive to use and easy to understand, then that would be fantastic and I would be very interested in seeing how it could be used. I'm looking at this from what we currently have for custom bbcodes, which doesn't come close to this sort of flexibility and requires this sort of logic be hard-coded, which we do not want. If the choice were between hard-coded logic such as this and code that doesn't perform any sort of sanity checking, e.g. for <b><i></b></i>, we would have to choose to not perform sanity checking to be able to move all of our bbcodes to custom ones. But if this can be done well in custom bbcodes go for it!

User avatar
Jacob
Registered User
Posts: 102
Joined: Wed Jan 04, 2012 1:41 pm

Re: [RFC] Integrate s9e\TextFormatter

Post by Jacob »

I agree with JoshyPHP. Blocking the post of the user is way overkill and would annoy the hell out of people.

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

Re: [RFC] Integrate s9e\TextFormatter

Post by JoshyPHP »

EXreaction wrote:If there is an easy way the custom bbcode editor can be built so this sort of logic is intuitive to use and easy to understand, then that would be fantastic and I would be very interested in seeing how it could be used.
Ultimately, my goal in generating rules automatically is that nobody would need to use it or think about it. Only the most advanced users should bother about the exact rules in place, like phpBB's default permission groups for example (nobody's second-guessing what kind of permissions the "Moderators" get.) I'll try to make time to document what kinds of rules are generated and how, it'll give you a clearer picture of what's going on. It's all basic stuff that people expect, it's meant to be unobtrusive.
EXreaction wrote:code that doesn't perform any sort of sanity checking, e.g. for <b><i></b></i>
Here's an interesting tidbit: s9e\TextFormatter cannot generate malformed HTML. Without any rules in place, [b]1[i]2[/b]3[/i] would be rendered as <b>1<i>2</i></b>3[/i]. And I'll double down with an interesting nugget: the auto-generated rules make it be rendered as <b>1<i>2</i></b><i>3</i>, which is precisely how an HTML5 browser would render <b>1<i>2</b>3</i>. You can try it in your browser's console:
console.png
console.png (3.38 KiB) Viewed 17921 times
EXreaction wrote:we would have to choose to not perform sanity checking to be able to move all of our bbcodes to custom ones
Actually it's pretty much the exact opposite. The default BBCodes require a bunch of rules, such as ommitting [/*], or not having a bunch of extra <br/> around quotes or inside list items. If they were not auto-generated you'd have to hardcode them, be it in PHP or stored in the database.

Anyway, I understand your position and I'm sure you'll be more comfortable once I'll document what rules are generated. Long term, I'll probably support configuring which rules get created and the ability to toggle whether to auto-generate rules on a per-tag basis.

User avatar
Pony99CA
Registered User
Posts: 986
Joined: Sun Feb 08, 2009 2:35 am
Location: Hollister, CA
Contact:

Re: [RFC] Integrate s9e\TextFormatter

Post by Pony99CA »

JoshyPHP wrote:Here's an interesting tidbit: s9e\TextFormatter cannot generate malformed HTML. Without any rules in place, [b]1[i]2[/b]3[/i] would be rendered as <b>1<i>2</i></b>3[/i]. And I'll double down with an interesting nugget: the auto-generated rules make it be rendered as <b>1<i>2</i></b><i>3</i>, which is precisely how an HTML5 browser would render <b>1<i>2</b>3</i>. You can try it in your browser's console:
console.png
We definitely shouldn't generate bad HTML, but I also want to avoid that former example getting posted (I hate seeing unrendered tags in posts).

Let's take your Bold/Quote example. Here's some code:

Code: Select all

Text  [b]Lots of text -- hundreds of characters

[quote]More text[/quote]

More text
So the ending [/b] is missing. What did the user intend? The fact is that you don't know. There are three simple rules that a parser could apply:
  1. End the Bold before the Quote tag.
  2. End the Bold before the Quote tag and put another Bold section around the text after the Quote tag.
  3. End the Bold before the Quote tag, put another Bold section in the Quote tag and put another Bold section around the text after the Quote tag.
However, what likely happened is that the user simply forgot to end the Bold section after a few words (maybe one) and none of the actions is what he really wanted, so he'd be shouting at the rest of us for a good part of his post. Worse,if he's too lazy to Preview his post, he's probably not going to look at it after he posts, either, so it will never get fixed.

That's why I want warnings and no posting. Of course, if you make that an option, the board owner can decide which fits his board better. For example, at Area 51, I would hope that everybody could understand a bad BBCode message, so there's no reason to not give the error here.

Steve
Silicon Valley Pocket PC (http://www.svpocketpc.com)
Creator of manage_bots and spoof_user (ask me)
Need hosting for a small forum with full cPanel & MySQL access? Contact me or PM me.

User avatar
Master_Cylinder
Registered User
Posts: 361
Joined: Wed Jul 31, 2013 9:54 pm

Re: [RFC] Integrate s9e\TextFormatter

Post by Master_Cylinder »

In the above example, since we can't know what the author intended, why not just remove the bold tag that has no /bold tag?
These kids today...
Buy them books, send them to school and what do they do?

They eat the paste. :lol:

User avatar
imkingdavid
Registered User
Posts: 1050
Joined: Thu Jul 30, 2009 12:06 pm

Re: [RFC] Integrate s9e\TextFormatter

Post by imkingdavid »

That would only confuse the user more. If the text comes out looking bold from the opening tag on, then the user at least knows "Oh, hey, I forgot to stop the bold formatting". If we remove it, he says... "ummm... what happened? This is so buggy".

Because we don't know what was intended, 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.

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.
I do custom MODs. PM for a quote!
View My: MODs | Portfolio
Please do NOT contact for support via PM or email.
Remember, the enemy's gate is down.

User avatar
Master_Cylinder
Registered User
Posts: 361
Joined: Wed Jul 31, 2013 9:54 pm

Re: [RFC] Integrate s9e\TextFormatter

Post by Master_Cylinder »

I see the point about removing it and I think I agree with just leaving it, as it is now. Users are used to it and if they see the they know to edit (if they can) or be more careful next time. 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?
These kids today...
Buy them books, send them to school and what do they do?

They eat the paste. :lol:

Post Reply