Opened 10 years ago

Closed 9 years ago

#5930 closed Bug (fixed)

htmldataprocessor: fix for convert style attributes to lowercase in IE

Reported by: Juergen Owned by: Sa'ar Zac Elias
Priority: Normal Milestone: CKEditor 3.5.1
Component: Core : Output Data Version: 3.0
Keywords: IE Cc: ckeditor@…

Description

File: _source/plugins/htmldataprocessor/plugin.js (Starting line 240)

The function to convert ALL styles to lowercase causing a problem with capital urls:

background-image: url(http://somedomain/SomeBackground.jpg);

is converted to:

background-image: url(http://somedomain/somebackground.jpg);

On a windows server, this is not the biggest problem, but a linux server won't find that file.

Heres the possible fix:

	if ( CKEDITOR.env.ie )
	{
		// IE outputs style attribute in capital letters. We should convert
		// them back to lower case.
		defaultHtmlFilterRules.attributes.style = function( value, element )
		{
			var res = value.match(/([a-z-]+):/gi);
			if (res) {
				for (var i = 0; i < res.length; ++i) {
					value = value.replace(res[i], res[i].toLowerCase());
				}
				return value;
			}
			else {
				return value.toLowerCase();
			}
		};
	}

Greetings, Juergen

Attachments (2)

5930.patch (758 bytes) - added by Sa'ar Zac Elias 10 years ago.
5930_2.patch (898 bytes) - added by Sa'ar Zac Elias 9 years ago.

Download all attachments as: .zip

Change History (14)

comment:1 Changed 10 years ago by Tobiasz Cudnik

Keywords: Confirmed HasPatch added; defaultHtmlFilterRules attributes style toLowerCase removed
Milestone: CKEditor 3.x

comment:2 Changed 10 years ago by Tobiasz Cudnik

I've been able to reproduce it, thank you for the fix. We will check it.

Changed 10 years ago by Sa'ar Zac Elias

Attachment: 5930.patch added

comment:3 Changed 10 years ago by Sa'ar Zac Elias

Keywords: Review? added
Owner: set to Sa'ar Zac Elias
Status: newassigned

comment:4 Changed 10 years ago by Charlie

This appears to be a larger problem with the CSSNormalizer. The lower case function also should not be applied to font names. See: http://cksource.com/forums/viewtopic.php?f=11&t=19446

I.E. Instead of outputting: style="font-family: comic sans ms,cursive;" We should get: style="font-family: Comic Sans MS, cursive;"

Since that's what we gave in the fonts plugin and it's possible for fonts to be case-sensitive on certain systems. (In general isn't CSS case-sensitive?)

comment:5 Changed 10 years ago by Charlie

ignore previous comment. Proposed patch fixes it (In the scope of this bug)

comment:6 Changed 10 years ago by Kingsquare

Cc: ckeditor@… added
Keywords: HasPatch added; HasPatch? removed
Version: 3.3.13.4.2

This problem still exists in 3.4.2. and again: the patch above fixes this problem.

Please apply this patch for the next release!

Thanks

comment:7 Changed 10 years ago by Sa'ar Zac Elias

Version: 3.4.23.0

Please don't change the version field.

comment:8 Changed 10 years ago by Wiktor Walc

Milestone: CKEditor 3.5.1

comment:9 Changed 9 years ago by Garry Yao

Status: reviewreview_failed

Could be even simplified?

return value.replace( /(^|;)([^\:]+)/g, function( match )
{
	return match.toLowerCase();
});

Changed 9 years ago by Sa'ar Zac Elias

Attachment: 5930_2.patch added

comment:10 Changed 9 years ago by Sa'ar Zac Elias

Status: review_failedreview

comment:11 Changed 9 years ago by Garry Yao

Keywords: Confirmed HasPatch removed
Status: reviewreview_passed

comment:12 Changed 9 years ago by Sa'ar Zac Elias

Resolution: fixed
Status: review_passedclosed

Fixed with [6294].

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