Vertical Alignment not removed

[noudadrichem]

PLEASE NOTE: make sure the bug exists in the latest patch level of the project. For instance, if you are running a 2.x version of Apostrophe, you should use the latest in that major version to confirm the bug.

To Reproduce

Step by step instructions to reproduce the behavior:

1. Go to 'html'
2. Click on 'terminal'
3. Scroll down to 'where it started'
4. See error

Expected behavior

My expected output is:
<font><font>SHOP NOW</font></font>
But got:
<font style="vertical-align:inherit"><font style="vertical-align:inherit">SHOP NOW</font></font>

Describe the bug

sanitizeHtml is not removing vertical-align from the HTML

Details

Version of Node.js:
PLEASE NOTE: Only stable LTS versions (10.x and 12.x) are fully supported but we will do our best with newer versions.
~/code/testing-sanitize » node -v
v10.8.0

Server Operating System:
MacOS Version 10.15.4

Additional context:
I created this code snippet to extract the problem from the bigger service.

const sanitizeHtml = require('sanitize-html');
const HTML = `<font style="vertical-align: inherit;"><font style="vertical-align: inherit;">SHOP NOW</font></font>`

function testVerticalAlignRemovement(html) {
    const clean = sanitizeHtml(html, {
        allowedTags: ['div', 'br', 'font', 'span', 'b', 'strong', 'i', 'u', '<strike>'],
        allowedAttributes: {
            'font': ['color', 'face', 'size', 'style'],
            'span': ['style']
        },
        allowedStyles: {
            'color': ['*'],
            'font-weight': ['*'],
            'font-style': ['*'],
            'text-transform': ['*'],
            'text-decoration': ['*'],
            'text-shadow': ['*']
        }
    });
    console.log(clean);
}

testVerticalAlignRemovement(HTML);

// expected output => <font"><font>SHOP NOW</font></font>

// got output => <font style="vertical-align:inherit"><font style="vertical-align:inherit">SHOP NOW</font></font>

Aka it didn't do anything..

Screenshots
Terminal issue:

<font style="vertical-align:inherit"><font style="vertical-align:inherit">SHOP NOW</font></font>```

[abea]

Please review the allowedStyles docs again. You need to include the element name (or '*' for the set of styles you're allowing. https://github.com/apostrophecms/sanitize-html#allowed-css-styles

[boutell]

Correct. The reason why it allows all styles right now is that you have font listed for "allowedAttributes" (correct and required for this) but you don't have it listed in "allowedStyles" (you are missing a level, see the docs as was mentioned).

…

On Wed, Jul 29, 2020 at 3:27 PM Alex Bea ***@***.***> wrote: Closed #391 <#391>. — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#391 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAAH27NUTDWOPUHQPKNJHGDR6BZZHANCNFSM4PLW4PBQ> .

-- THOMAS BOUTELL | CHIEF TECHNOLOGY OFFICER APOSTROPHECMS | apostrophecms.com | he/him/his

[noudadrichem]

I want vertical-alignment to be gone in the style attribute on font. So it not being there in 'allowedStyles' should remove it. That's how I read the docs as well. (see expected output)

[noudadrichem]

I got it working by adding this:

        allowedStyles: {
            '*': {
                'color': ['*'],
                'font-weight': ['*'],
                'font-style': ['*'],
                'text-transform': ['*'],
                'text-decoration': ['*'],
                'text-shadow': ['*'],
            }
        }