Opened 15 years ago

Closed 14 years ago

#4589 closed Bug (expired)

Output style attribute "override" by regular expression is ignored

Reported by: pomu0325 Owned by: Garry Yao
Priority: Normal Milestone: CKEditor 3.2
Component: Core : Styles Version: 3.0
Keywords: Cc:

Description

For example, following override setting unintendedly matches all <font> element whatever attribute it has.

config.fontSize_style =
{
	element		: 'span',
	attributes	: { 'class' : '#(size)' },
	overrides	: [ { element : 'font', attributes : { 'class' : /^foo/ } } ]
};

'CKEDITOR.tools.clone()' seems not to clone RegExp object properly at line.89 of styles/plugin.js

styleDefinition = CKEDITOR.tools.clone( styleDefinition );

After this line, regular expression is turned into , therefore it matches anything.

Attachments (3)

4589.patch (422 bytes) - added by pomu0325 15 years ago.
4589_2.patch (784 bytes) - added by Garry Yao 15 years ago.
4589_3.patch (540 bytes) - added by Garry Yao 14 years ago.

Download all attachments as: .zip

Change History (10)

comment:1 Changed 15 years ago by pomu0325

It seems to be a side-effect of [3704], 'clone' method now doesn't clone RegExp properly.

reproduce

  • Add above settings to config.js
  • Open any CKEditor sample.
  • Paste following html in source mode.
    <font class="foo"><font class="bar">test</font></font>
    
  • Select text and choose font-size dropdown in WYSIWYG mode

expected result

Only the <font> element which matches the regular expression should be overrided.

<span class="16px"><font class="bar">test</font></span>

actual result

Although 'class="bar"' doesn't match the regular expression, it is also overrided.

<span class="16px">test</span>

This problem is simply avoided by just cloning RegExp object. Is there any reason RegExp should not be cloned??

Changed 15 years ago by pomu0325

Attachment: 4589.patch added

comment:2 Changed 15 years ago by Wiktor Walc

Keywords: HasPatch added

comment:3 Changed 15 years ago by Frederico Caldeira Knabben

Milestone: CKEditor 3.2

The patch should be good for it.

Changed 15 years ago by Garry Yao

Attachment: 4589_2.patch added

comment:4 Changed 15 years ago by Garry Yao

Owner: set to Garry Yao
Status: newassigned

Beside the RegExp fix there, the array clone could be further simplified - Array is simply first class Object in JavaScript.

comment:5 Changed 15 years ago by Garry Yao

Keywords: Review? added; HasPatch removed

comment:6 Changed 14 years ago by Alfonso Martínez de Lizarrondo

Keywords: Review- added; Review? removed

The RegExp part has been merged in [4774] from the 3.1.x branch, so that part isn't needed.

I've done a quick test and it seems that the behavior remains the same if the code for the Array object is removed. Maybe it was added due to problems with some browser? If no one finds problems with that I guess that it could be removed.

Changed 14 years ago by Garry Yao

Attachment: 4589_3.patch added

comment:7 in reply to:  6 Changed 14 years ago by Garry Yao

Keywords: Review- removed
Resolution: expired
Status: assignedclosed

Replying to alfonsoml:

The RegExp part has been merged in [4774] from the 3.1.x branch, so that part isn't needed.

The problem described at this ticket was fixed already by [4774].

...Maybe it was added due to problems with some browser?

It's not targeting a specific browser, just to make the array clone codes simpler.

Note: See TracTickets for help on using tickets.
© 2003 – 2022, CKSource sp. z o.o. sp.k. All rights reserved. | Terms of use | Privacy policy