Opened 4 years ago

Last modified 4 years ago

#12853 confirmed Bug

Removing HTML comment nodes with dataProcessor.htmlFilter removes script nodes too

Reported by: Túbal Owned by:
Priority: Normal Milestone:
Component: Core : Output Data Version:
Keywords: Cc:

Description

I've pasted the code below so you can see what I'm doing to strip HTML comments from the HTML code. The HTML comments get effectively stripped out but empty script tags (<script src="..."></script>) get stripped out too:

CKEDITOR.on('instanceReady', function(ev) {
  // Filter rules to apply on output
  // Node properties & methods: http://docs.ckeditor.com/#!/api/CKEDITOR.htmlParser.element-property-attributes
  ev.editor.dataProcessor.htmlFilter.addRules({
    comment: function() {
      // Remove HTML comments
      return false;
    },
    elements: {
      script: function(node) {
        // Remove language and script attributes from script tags
        'language' in node.attributes && delete node.attributes.language;
        'type' in node.attributes && delete node.attributes.type;
      }
    }
  });
});

I fixed it just by commenting out the "comment" rule in the htmlFilter. A bit weird :S

Change History (2)

comment:1 Changed 4 years ago by Piotrek Koszuliński

Keywords: comment script filter remove removed
Status: newconfirmed
Version: 4.4.6

That's because before content is parsed scripts are turned into comments, so they are not executed at any point. That's the only reasonable way to keep them while editing, but to block their execution.

The question that you may ask may be - why is it done before parsing (thus before applying the filter) and not later? The answer is that before the content is parsed it is passed through a temporary element (we set innerHTML and read it) so incorrect HTML is slightly improved by the browsers' native mechanisms.

Moreover, to handle scripts we're using the same mechanism as for config.protectedSource. For the latter to work the encoding into comments must be done before parsing, because otherwise that content wouldn't be protected and could be incorrectly parsed since it may not be valid HTML.

So it is a pretty complex situation. We could perhaps improve it, but the cost of implementation as well as for the performance would be high, so we don't plan to do this.

comment:2 Changed 4 years ago by Túbal

OK guys, thank you for such a detailed explanation ;) Keep up the good work!

Note: See TracTickets for help on using tickets.
© 2003 – 2019 CKSource – Frederico Knabben. All rights reserved. | Terms of use | Privacy policy