Opened 10 years ago

Closed 10 years ago

#2754 closed Bug (fixed)

CKEditor v3 enters infinite loop when trying to output HTML code

Reported by: Martin Kou Owned by: Martin Kou
Priority: Must have (possibly next milestone) Milestone: CKEditor 3.0
Component: Core : Output Data Version: SVN (FCKeditor) - Retired
Keywords: Confirmed Review+ Cc:

Description (last modified by Martin Kou)

To reproduce the bug:

  1. Open replacebyclass.html in the samples in Firefox 3.
  2. Also open http://gplv3.fsf.org/ in Firefox 3.
  3. Copy everything in FSF's page into the editing area.
  4. Click "Source" - the browser freezes.

This is not a performance problem with the new code - I've tried pasting the long text of GPL v3 in the editor area and the speed of switching to source mode is noticeable faster than v2.

Preliminary tests showed that the infinite loop is from the HTML data processor plugin.

Attachments (2)

2754.patch (601 bytes) - added by Martin Kou 10 years ago.
2754_2.patch (599 bytes) - added by Martin Kou 10 years ago.

Download all attachments as: .zip

Change History (12)

comment:1 Changed 10 years ago by Artur Formella

Keywords: Confirmed added

comment:2 Changed 10 years ago by Martin Kou

Description: modified (diff)

comment:3 Changed 10 years ago by Martin Kou

Owner: set to Martin Kou
Status: newassigned

comment:4 Changed 10 years ago by Martin Kou

Here's the HTML string in the page that causes the freeze:

<div id="portal-colophon">
      <!--

        Please keep the Plone Powered button (or a textual link to us) if you use
        Plone on your site. It's a nice token of gratitude, and we appreciate your
        help in promoting the Plone name.

        Plone is powered by the combined forces of Zope and CMF, two absolutely
        great systems made by Zope Corporation (http://zope.com) and they in turn
        are based on the best programming language in the world - Python
        (http://www.python.org). We owe these guys a lot, thanks for making Plone
        possible!

      -->

      <a href="http://plone.org" class="colophonIcon"
         title="This Plone site was built using Plone. Click for more information."
         style="background-image: url(http://gplv3.fsf.org/plone_powered.gif);">
        Powered by Plone
      </a>
</div>

comment:5 Changed 10 years ago by Martin Kou

Priority: NormalHigh

Put to high priority for the beta release.

Changed 10 years ago by Martin Kou

Attachment: 2754.patch added

comment:6 Changed 10 years ago by Martin Kou

The loop was found to be caused by the HTML parser's "htmlParts" regular expression - it doesn't expect comments to contain newlines.

comment:7 Changed 10 years ago by Martin Kou

Keywords: Review? added

comment:8 Changed 10 years ago by Frederico Caldeira Knabben

Keywords: Review- added; Review? removed

Martin, the correct replacement for . is [\S\s], not (?:\s|\S). In this way you replace a character set reference by another character set reference, instead of a matching group (performs better, theoretically).

Changed 10 years ago by Martin Kou

Attachment: 2754_2.patch added

comment:9 Changed 10 years ago by Martin Kou

Keywords: Review? added; Review- removed

comment:10 Changed 10 years ago by Frederico Caldeira Knabben

Keywords: Review+ added; Review? removed
Resolution: fixed
Status: assignedclosed

I've committed the patch with [2918]. I did that after the review, to avoid delay in the release.

I've changed that part of the regex to [\S\s], instead of [\s\S]. You gonna think I'm crazy, but as I can't explain precisely how browsers implement their regex engines, I'm assuming the case where that match means "find a character from the \S group (first check) or a character from the \s group (second check)". So, as we have more characters than spaces inside comments (usually), there is a theoretical performance difference by changing the order the characters are presented into the square brackets. And yes... you really gonna think I'm crazy.

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