csslint.js: @page rule fatal error with padding-*/margin-*

[Code-yWilliams]

Describe the bug

CSSLint throws a fatal error when the very first parsed rule is @page and it includes one of the following properties: margin-top, margin-bottom, margin-left, margin-right, padding-top, padding-bottom, padding-left, padding-right. It crashes the worker and prevents the linting of all other css in the document.

e.g.

    @page: {
        padding-left: 50px;
    }

Expected Behavior

I expect the linter not to break. I expect it to accept the @page rule with one of the problematic properties without issue. It should continue parsing and linting the remainder of the css that is passed to it.

Current Behavior

Lines 9756 - 9762 of ACE/lib/ace/css/csslint.js define an event listener for the property event, which fires correctly.

        parser.addListener("property", function(event) {
            var name = event.property.toString().toLowerCase();

            if (propertiesToCheck[name]) {
                properties[name] = 1;
            }
        });

The purpose of this listener is to check for usage of the padding-* and margin-* properties, which explains why this issue is specific to those properties. when the first rule of the document is @page, the properties variable is undefined when this event handler runs As a result, properties[name] = 1 raises TypeError: Cannot set properties of undefined

This error is eventually caught by lines 7958- 7960:

        catch (ex) {
            reporter.error("Fatal error, cannot continue: " + ex.message, ex.line, ex.col, {});
        }

This breaks the linter, preventing all remaining rulesets from being parsed.

Reproduction Steps

Set the ace editor mode to ace/mode/css

    const editorElement = document.getElementById('editor');
    const editor = ace.edit(editorElement, {
        value: initialCss,
    });
    editor.session.setOptions({
        mode: 'ace/mode/css',
    });

Enter the following rule into the editor (make sure it is the first and only one in the parsed CSS):

    @page {
        padding-top: 50px;
    }

Any syntax highlighting will be red, indicating the presence of the fatal error (fatal error is not displayed in the gutter like normal css syntax errors).

Below the @page rule, add an additional problematic css rule (e.g. fake-rule: }}})

Note how the linter worker is not capable of linting this additional rule because it has crashed.

Possible Solution

Add a default value to the properties variable on line 10649:

    init: function (parser, reporter) {
      'use strict'
      var rule = this,
        prop,
        i,
        len,
        propertiesToCheck = {},
        properties, // <----- HERE
        mapping = {
          margin: [
            'margin-top',
            'margin-bottom',
            'margin-left',
            'margin-right',
          ],
          padding: [
            'padding-top',
            'padding-bottom',
            'padding-left',
            'padding-right',
          ],
        }

This problem goes away when there is a default empty object:

    init: function (parser, reporter) {
      'use strict'
      var rule = this,
        prop,
        i,
        len,
        propertiesToCheck = {},
        properties = {}, // <----- HERE
        mapping = {
          margin: [
            'margin-top',
            'margin-bottom',
            'margin-left',
            'margin-right',
          ],
          padding: [
            'padding-top',
            'padding-bottom',
            'padding-left',
            'padding-right',
          ],
        }

Because this is specific to when @page is the first rule of the document, this default value is a practical solution. Alternatively, there might be solution that correctly initializes the properties object when the first rule of the document is an @page rule.

Additional Information/Context

Line 1496 fires the startpage event. The event listener for startpage executes the startRule function, which should initialize an empty properties object. Still, somehow, properties is undefined, causing this fatal error.

It appears that something is preventing the initialization of the properties object when the first rule is @page.

Ace Version / Browser / OS / Keyboard layout

1.23.1 / Chrome 114.0.5735.133 / macOS Ventura 13.4.1 / qwerty US

[akoreman]

hey, thanks for opening the issue. I'm trying to reproduce the issue on the kitchen-sink but I don't see the issue on my end. Can you reproduce the issue on the kitchen-sink?

(I did notice that on your reproduction snippet, there is a : after the @page which I don't think should be there?)

[Code-yWilliams]

@akoreman Hi there! I am able to recreate it on the kitchen sink (you're right about the : in the reproduction snippet, but the issue occurs even without it).

The fatal error is not rendered in the gutter like other syntax errors. The only way to tell that it broke is to write more text below the @page rule and you will see that there is no further linting taking place. If you enter this:

@page {
    padding-top: 50px;
}
abc123xyz///}}}

It is not showing any errors in the gutter despite the incorrect syntax on line 4 because the worker is no longer capable of linting after it parses the @page rule.

Notice how if you enter the same mess of text on line 1 before the @page rule, there is an error icon in the gutter as expected:

I hope that helps, but please let me know if you are still not getting the same result. Thanks!

[akoreman]

ah yeah I see the issue now, thanks.

[github-actions[bot]]

This issue has not received any attention in 1 year. If you want to keep this issue open, please leave a comment below and auto-close will be canceled.