BBCode [size=] tag doesn't work in 2.6.4

pursuit

I upgraded from 2.2.1 to 2.6.4 and solved most of the ensuing issues.

Various BBCode tags that didn't work in previous installation now started working. However, the size tag does nothing now. Well, it does something in fact, as it is not visible in the comment text. So at least it is being parsed and replaced with something, it would seem. Doesn't affect the [size=7]size[/size] [size=1]of the text though.[/size]

After examining /library/core/class.bbcode.php and /vendor/vanilla/nbbc/src/BBCodeLibrary.php I can't figure out the reason of this behavior.

The tag has been used extensively on our forum. Is there any way to fix it? I'm ready to edit core files if needed.

pursuit

I temporarily fixed this by using nbbc doSize function as template in class.bbcode.php and substituting <span> with <big> for each case. It's a very crude solution as you can see.

It seems like return "<span style=\"font-size:{$size}\">{$content}</span>"; in the default class.bbcode.php doSize function won't accept font-size or class. I can modify it to change font color and such, but not size for some reason.

R_J

The problem is quite complex and when I read that you are willing to alter core files I thought it will be hard to tell you not to do so. But frankly spoken, I do not see a way to do so without changing core files...

The problem is not the BBCode parsing that happens exactly as you expect. The problem ist that the resulting HTML is cleaned up (which is a good idea) but the only allowed style element is "color", but not "size".
Take a look at the class.format.php:

   public static function bbCode($mixed) {
        if (!is_string($mixed)) {
            return self::to($mixed, 'BBCode');
        } else {
            // See if there is a custom BBCode formatter.
            $bBCodeFormatter = Gdn::factory('BBCodeFormatter');
            if (is_object($bBCodeFormatter)) {
                // Standard BBCode parsing.
                $mixed = $bBCodeFormatter->format($mixed);

                // Always filter after basic parsing.
                // Add htmLawed-compatible specification updates.
                $options = [
                    'codeBlockEntities' => false,
                    'spec' => [
                        'span' => [
                            'style' => ['match' => '/^(color:(#[a-f\d]{3}[a-f\d]{3}?|[a-z]+))?;?$/i']
                        ]
                    ]
                ];
                $sanitized = Gdn_Format::htmlFilter($mixed, $options);

Before the last line everything is as you would expect it (just insert decho($mixed); above to check it).

YOu would have to change that line with the regex ('style' => [...) but don't ask me how.

Linc

We removed 'size' as a feature due to accessibility issues. Allowing members to dictate the size at which others read text is a bad experience for low-vision users, as well as users with reading challenges.

R_J

@Linc said:
We removed 'size' as a feature due to accessibility issues. Allowing members to dictate the size at which others read text is a bad experience for low-vision users, as well as users with reading challenges.

Is there a way to influence that filter options? The HTML filter options are very rigid as far as I can tell. In VanillaHtmlFormatter I can see

    public function format($html, $options = []) {

        $attributes = c('Garden.Html.BlockedAttributes', 'on*, target, download');
...
        if (c('Garden.Format.DisableUrlEmbeds')) {

But that's really all that I would see which can be changed in this process.

The only "clean" way I can theoretically think of would be to register a custom HtmlFormatter that extends the original Htmlawed and overrides the filter method to alter the $spec array before it calls the parents filter method to either disallow even the color option or allow more options.

But a) this is overkill and b) would always be used when the HtmlFilter is used and wouldn't be connected to BBCode parsing which only might be good enough.

Seeing problems which can only be solved by changing core files is somewhat unsatisfying...

Linc

@R_J said:
Seeing problems which can only be solved by changing core files is somewhat unsatisfying...

Folks seeing it as a problem that we prioritized accessibility over tweaking is unsatisfying.

I'd assume there's some way to work around it, but I'm not super interested in spending time puzzling over how to mess with folks' reading experience.

pursuit

@R_J said:
YOu would have to change that line with the regex ('style' => [...) but don't ask me how.

Thanks, I might try this solution when browsers stop supporting <big> tag. For now I'll stick to my crude code, as it does the trick, and regex can be difficult to properly implement.

@Linc said:
We removed 'size' as a feature due to accessibility issues. Allowing members to dictate the size at which others read text is a bad experience for low-vision users, as well as users with reading challenges.

Uh, that's quite an unexpected approach. You do realize the same could be said about 'color' feature (color blindness is a thing) and a number of other formatting options?

Why not leave that decision to administrators, making it possible to adjust accessibility according to specific needs of their communities?

@Linc said:
I'd assume there's some way to work around it, but I'm not super interested in spending time puzzling over how to mess with folks' reading experience.

Meanwhile the update to 2.6.4 broke our reading experience by making thousands of bbcode-formatted headings difficult to distinguish in the flow of text (and our maximum comment length is greatly increased for good reasons). And now I'm spending another day puzzling over how to make it read like before.

In other words your change messes with folk's reading experience already. Ironic.

R_J

Try using this line instead of the above:
'style' => ['match' => '/^(color:(#([a-f\d]{3}){1,2}|[a-z]+])?);?|font-size:\d+(.\d+)?em;?/i']

pursuit

Thanks!

It seems to work fine and allows for much better precision than my temporary fix. So I guess I'll just use it from now on instead of waiting till the <big> tag goes of style

Do I understand it correctly that the {1,2} part is for matching either three- or six-digit hex codes (and is effectively the same as the original regex)?

R_J

Correct. I started by trying to understand the existing regex and I found it more readable that way, but I guess this is just a matter of taste.

Linc

@pursuit said:
You do realize the same could be said about 'color' feature (color blindness is a thing) and a number of other formatting options?

We don't support the 'color' option in the our next-generation editor. That said, color blindness rarely if ever inhibits a person from reading. That's more an aesthetics argument.

@pursuit said:
In other words your change messes with folk's reading experience already. Ironic.

Preferences and abilities are not the same thing.

Note we still support the heading tags, which (indirectly) allow for differentiated sizes in a way that works for everyone.

pursuit

@Linc said:
We don't support the 'color' option in the our next-generation editor.

That's a major argument against upgrading for any forum that includes color coded content. While I can understand not prioritizing backwards compatibility of plugins and such, backwards compatibility of forum content should be serious consideration, I think. Nobody wants to have to go back and reformat ten thousands posts after an upgrade.

I totally don't buy your concept that text accessibility is an engine-level issue rather than administration-level issue. Not when the purpose of software in question is allowing people to process and present text. By limiting options in this area you are limiting the range of possible applications.

@Linc said:
Preferences and abilities are not the same thing.

When it comes to long and dense articles we are generally unable to locate standard font size headings in a wall of text. That's not a preference. That's how human sight works, and some have it tougher than others.

@Linc said:
Note we still support the heading tags, which (indirectly) allow for differentiated sizes in a way that works for everyone.

Is there a BBCode for heading?

R_J

@Linc said:
Note we still support the heading tags, which (indirectly) allow for differentiated sizes in a way that works for everyone.

Headings would be more semantic but [heading]This is a heading[/heading] is not even parsed.

@pursuit I think I have some errors in my regex. I also added font face options to it and ended up with this:

                $fontColor = 'color:(#([a-f\d]{3}){1,2}|[a-z]+)';
                $fontSize = 'font-size:\d{1,2}(\.\d{1,2})?em';
                $fontFamily = "font-family:'[a-zA-Z ]+'";
                $fontMatch = "/^({$fontColor}|{$fontSize}|{$fontFamily});?$/i";
                $options = [
                    'codeBlockEntities' => false,
                    'spec' => [
                        'span' => ['style' => ['match' => $fontMatch]]
                    ]
                ];

charrondev

We don't support the 'color' option in the our next-generation editor. That said, color blindness rarely if ever inhibits a person from reading. That's more an aesthetics argument.

This is the editor now enabled on this forum.

As far modifying core files I would recommend against, although like most of our product the HTMLFilterer is setup through dependency injection so an addon could extend it however you like and replace the built in one in the container.

https://docs.vanillaforums.com/developer/framework/dependency-injection/#example

@R_J went into this a little bit but I could elaborate slightly.

$dic
  ->rule('HtmlFormatter')
  ->setClass(VanillaHtmlFormatter::class)

That's our built in declaration so the addon should probably extend extend that class and override the built in one. That has the affect of replacing that piece across the entire application without forking the app itself (and making it very difficult to upgrade).

pursuit

So, @charrondev and @@R_J, any more specific suggestions on how to turn this into a plugin?

I'm currently in the process of moving most of my core changes into addons. I have them all documented, but as their number grew they became less easy to manage that way. Still using 2.6.4, but not sure how long it will be viable, so preparing for possible another future update seems sensible at this point.

I'm not very good with addons though. Not sure how to approach this one.

R_J

There are two major issues: first is that a plugin for that purpose created for Vanilla 2.6 will most probably not work with 3.3 and vice versa. The second problem is that the solution suggested above doesn't work as I would expect. I've created two files, one plugin and one formatter class:

class MyBBCodePlugin extends Gdn_Plugin {
    public function container_init($container) {
        $container->rule('HtmlFormat')->setClass('MyHtmlFormat');
    }
}

class MyHtmlFormat extends HtmlFormat {
    public function renderHtml(string $content, bool $enhance = true): string {
        return 'dummy';
    }
}

But MyHmlFormat isn't used

pursuit

Well, I guess I'm left with a few core files that will stay directly modified. Seven files specifcally, including the two modified to make this thing work. Not a lot of changed lines of code to account for during an update all in all.

toolsmythe

https://open.vanillaforums.com/discussion/comment/255320#Comment_255320

With all due respect, I think you folks are building limitations into the core that should be options left to administrators. If someone wants to over-use color to the point where the UI is angry fruit salad, that should be their choice.

Set reasonable defaults by all means, but remove functionality because YOU think it's right? 😧