[BUG] Multiple font-face at-rule CSS rules are not processed

[tommedema]

Latest version of grapes #cfde1c1

If you supply a single @font-face, this is parsed just fine:

JSFiddle example: https://jsfiddle.net/szLp8h4n/140/

Screenshot:

However, if you supply a second @font-face, only the second one is parsed, and the first is lost:

JSFiddle example: https://jsfiddle.net/szLp8h4n/141/

Screenshot:

I am trying to debug this but would appreciate your help. Could it be related to line 122 in ParserCss.js?

[tommedema]

@artf FYI I can now reproduce this in a failing test case:

it('Init editor from element with multiple font-face at-rules', () => {
      config.fromElement = 1;
      config.storageManager = { type: 0 };
      fixture.innerHTML = `
      <style>
        @font-face {
          font-family: 'Glyphicons Halflings';
          src: url(https://cdnjs.cloudflare.com/ajax/libs/twitter-bootstrap/3.3.7/fonts/glyphicons-halflings-regular.woff2) format('woff2');
        }
        @font-face {
          font-family: 'Droid Sans';
          src: url(https://fonts.gstatic.com/s/droidsans/v8/SlGVmQWMvZQIdix7AFxXkHNSbRYXags.woff2) format('woff2');
        }
      </style>` + htmlString;
      const editor = obj.init(config);
      const css = editor.getCss();
      const styles = editor.getStyle();
      expect(styles.length).toEqual(2);
    });

After further debugging this seems to be caused by cssComposer thinking that the second rule is the same rule as the previous one and then returning the first rule:

https://github.com/artf/grapesjs/blob/dev/src/css_composer/index.js#L185

[cjpollard]

I managed to fix this in my fork here. Not sure if this is the best way of doing it, but I can submit a pull request if desired.

[tommedema]

@cjpollard thanks, but it seems like you are taking apart something that was stitched together for the wrong reason.

I believe a better approach is to not stitch together these at-rules in the first place. @artf I have submitted a PR for this:

#1036

To be honest, in my opinion the parsers should not try to be smart and optimize code. The browser is a parser itself, and if the rules are found in the browser, grapes should honor this. By making the parser complicated we introduce several bugs (I myself have gathered a list already). In fact I'd recommend not having a parser at all and just getting components and their styles etc. in real time from the DOM / Javascript API.

[artf]

Totally agree with you Tom, the main goal of cssComposer is just to provide a lean interface for style definitions. What we have inside CssParser right now it's actually a browser's parser and the rest of code is just a traverser. Actually, we have issues like #307 which show the inconsistency generated by browser's parsers (indeed you will get different outputs on each browser).
My idea (added alredy to the Roadmap #74) was to create an extendable interface which could allow adding custom parsers to the editor (eg. postcss parser for CSS) but obvisosly didn't yet find the time to start it.

BTW thank you for such a quick solution

[tommedema]

@artf I see, but then you probably shouldn't do things like removing empty tags or concatenating the same rules; that's extending your "traverser" with parser capabilities ;)

[lock]

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.