Opened 6 years ago

Last modified 19 months ago

#10021 confirmed Bug

Table plugin uses attributes deprecated in HTML5

Reported by: Danil Owned by:
Priority: Nice to have (we want to work on it) Milestone:
Component: Core : Tables Version:
Keywords: Cc:

Description (last modified by Piotrek Koszuliński)

When I insert new table, without any settings i get border="1" cellpadding="1" cellspacing="1" style="width: 500px;" attributes. These are unexpected and non-valid ones.

Edit:
When I insert new table it uses attributes that are deprecated in HTML5 cellpadding,cellspacing,align,summary,rules,frame,bgcolor attributes. Please see: http://www.w3schools.com/tags/tag_table.asp


Easy workaround (aka solution): comment:13.

Attachments (3)

table.js.diff (10.4 KB) - added by Stephen 4 years ago.
diff with table 4.4.6
fix_ckeditor_table_js.diff (13.8 KB) - added by Stephen 4 years ago.
table.js (27.2 KB) - added by Stephen 4 years ago.
html 5 version of table plug in

Download all attachments as: .zip

Change History (30)

comment:2 Changed 6 years ago by Danil

How can I get rid of these deprecated attributes? I use stylesheet file to style plain tables (I guess most modern sites use it). Is any settings to tweak table plugin? It should at least use css to have any effect.

comment:3 Changed 6 years ago by Jakub Ś

Description: modified (diff)
Summary: Insert table plugin adds unexpected attributesTable plugin uses attributes deprecated in HTML5

comment:4 Changed 6 years ago by Jakub Ś

Description: modified (diff)

comment:5 Changed 6 years ago by Jakub Ś

Component: Core : EditableCore : Tables
Description: modified (diff)
Status: newconfirmed

@danya_postfactum I have modified your description as you are right - CKEditor uses some old and deprecated attributes that should be replaced by styles in HTML5.

  1. When creating table one can always delete these values in table dialog (manually set values to zero or delete them)
  2. These default values are set in dialog plugin and the only way to change them is by setting them to empty values:
    CKEDITOR.on( 'dialogDefinition', function( ev )	{				
    				var dialogName = ev.data.name;
    				var dialogDefinition = ev.data.definition;						
    				if ( dialogName == 'table' ){
    					var infoTab = dialogDefinition.getContents( 'info' );
    					infoTab.get( 'txtBorder' ).default = '';
    					infoTab.get( 'txtWidth' ).default = '';
    					infoTab.get( 'txtCellSpace' ).default = '';
    					infoTab.get( 'txtCellPad' ).default = '';
    				}
    			}	);
    

comment:6 Changed 6 years ago by Danil

Thanks, this is more correct info. But I still think default width should be null.

comment:7 Changed 6 years ago by Jakub Ś

Description: modified (diff)
  1. These are not used by editor: rules,frame,bgcolor
  2. This can probably be dropped: summary
  3. This can perhaps be replaced by float in style attribute: - align
  4. There is a problem with: cellpadding and cellspacing

We still support IE7 and Quirks mode which makes this request very nice but impossible to implement at the given moment at least for these two attributes

Links: http://www.quackit.com/css/css_cellpadding.cfm
http://www.quackit.com/css/css_cellspacing.cfm
http://stackoverflow.com/questions/339923/how-to-set-cellpadding-and-cellspacing-in-css
http://www.w3schools.com/cssref/pr_tab_border-spacing.asp

Last edited 6 years ago by Jakub Ś (previous) (diff)

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

Please, be very careful before forcing this change for everybody.
Something similar was decided to change the dimensions in images and it has been an ongoing source of frustration for too many people.

And just testing a little shows that not setting a default width for a table makes it quite hard for the users to edit it easily (vs having a default with).

comment:9 Changed 6 years ago by Jakub Ś

@alfonsoml - good point. The ticket you have mentioned is #5547 and yes lots of users have complained that e.g. outlook doesn't accept width and height in styles but only as attributes.
You are 100% right, before implementing this some heavy research should be made. Idea looks interesting and modern but editor should also coexist with other apps or tools.

comment:11 Changed 6 years ago by Jakub Ś

#10688 was marked as duplicate.

comment:12 Changed 5 years ago by Piotrek Koszuliński

cc

comment:13 Changed 5 years ago by Piotrek Koszuliński

Description: modified (diff)

BTW. Since CKEditor 4.4 this is possible:

CKEDITOR.replace( 'editor1', {
	disallowedContent: 'table[cellspacing,cellpadding,border]'
} );

Fields are removed from the dialog and content is filtered on input. This works even in ACFs automatic mode, so no need to redefine entire allowed content rules.

Read more in the Disallowed Content guide.

comment:14 Changed 4 years ago by Danil

border=1 is good. But it's setting should be a checkbox: "borders: yes/no" width: 500px is bad. Any magic number is bad. Content styles usually set width to 100%.

Width:500px almost never suits real needs of content editors.

comment:15 Changed 4 years ago by Jakub Ś

Width:500px almost never suits real needs of content editors.

If you weren't focused only on complaining, you would have read comment:5 where it is shown how can you change default value for width. Instead of empty value you can assign 100% there.

Personally I agree with 100% approach but it doesn't mean that editor is broken because you can very easily adjust this behaviour to your needs.

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

Actually, there's no good value. We had width 100% as a default, but those who want to align table to the left or right were complaining. Also, the table was always stretched what doesn't make sense in many cases. What's more, setting nothing is even worse, because an empty table is very narrow, so it's hard to click the right cells.

On the other hand, I have to agree that width 500px is not a good choice too... Perhaps it's even worse than setting 100% always. We can reconsider removing that patch, but we would have to check whether there were some other reasons.

comment:17 Changed 4 years ago by Jakub Ś

In the meantime please use:

For removing old HTML4 fileds:

CKEDITOR.replace( 'editor1', {
	disallowedContent: 'table[cellspacing,cellpadding,border]'
} ); 

For setting width to default 100%

CKEDITOR.on( 'dialogDefinition', function( ev )	{				
	var dialogName = ev.data.name,
	dialogDefinition = ev.data.definition;						
	if ( dialogName == 'table' ){
		var infoTab = dialogDefinition.getContents( 'info' );
		infoTab.get( 'txtWidth' ).default = '100%';
	}
});

comment:18 Changed 4 years ago by Danil

TinyMCE inserts by default clean <table> without attributes. It's also resizable by default. I think it's best default behaviour. If site developer has defined table width in contents css, it can disable resizing feature.

Changed 4 years ago by Stephen

Attachment: table.js.diff added

diff with table 4.4.6

comment:19 Changed 4 years ago by Stephen

I have made an html5 version of the table plug in, will attach:

changes to en.js:
..."width":"Width","height":"Height","align":"Alignment","alignLeft":"...
==>
..."width":"Width","height":"Height","borderstyle":"Border Style","borderstyleNone":"none","borderstyleSolid":"solid","borderstyleDotted":"dotted","borderstyleDashed":"dashed","borderstyleHidden":"hidden","borderstyleDouble":"double","borderstyleGroove":"groove","borderstyleRidge":"ridge","borderstyleInset":"inset","borderstyleOutset":"outset","borderstyleInitial":"initial","borderstyleInherit":"inherit","align":"Alignment","alignLeft"...


also:

"chooseColor":"Choose"},"cellPad":"Cell padding"

++>

"chooseColor":"Choose"},"borderColor":"Border Color","cellPad":"Cell padding"



in ckeditor.js I have:

allowedContent:"td th{border,padding, border-width,border-style,padding,width,height,border-color,background-color,white-space,vertical-align,text-align}[colspan,rowspan]

allowedContent:"table{border-spacing,border,border-collapse,border-spacing,border,float,height,width,border-style}[align,border,cellpadding,cellspacing,summary];




 





Version 1, edited 4 years ago by Stephen (previous) (next) (diff)

comment:20 in reply to:  19 Changed 4 years ago by Stephen

Last edited 4 years ago by Stephen (previous) (diff)

comment:21 Changed 4 years ago by Stephen

I had issues with the full package, this worked with standard so modded a bit:

in ckeditor.js I have:

allowedContent:"td th{border,padding,border-width,border-style,padding,width,height,border-color,background-color,white-space,vertical-align,text-align}[colspan,rowspan]

allowedContent:"td th{border,padding,border-width,border-style,padding,width,height,border-color,background-color,white-space,vertical-align,text-align}[colspan,rowspan]

in en.js I have:

"width":"Width","height":"Height","borderstyle":"Border Style","borderstyleNone":"none","borderstyleSolid":"solid","borderstyleDotted":"dotted","borderstyleDashed":"dashed","borderstyleHidden":"hidden","borderstyleDouble":"double","borderstyleGroove":"groove","borderstyleRidge":"ridge","borderstyleInset":"inset","borderstyleOutset":"outset","borderstyleInitial":"initial","borderstyleInherit":"inherit","align":"Alignment","alignLeft"


Changed 4 years ago by Stephen

Attachment: fix_ckeditor_table_js.diff added

Changed 4 years ago by Stephen

Attachment: table.js added

html 5 version of table plug in

comment:22 Changed 4 years ago by Stephen

aaaahhh one more edit in table:

label: editor.lang.table.tablebordercolor,

==>

label: editor.lang.common.tablebordercolor,

comment:23 Changed 4 years ago by siirila

It looks like this is still using the older HTML attributes with the latest 4.5.3 table plugin. devorobinson, I tried your patched version, but I still didn't get the borders to appear correctly. For some reason it never actually set the border-width property on the table, so no child elements were getting their border-width set either.

This is a problem because the CSS Spec states that any non-CSS presentation hints, such as the border attribute, have a specificity of 0 so literally any CSS rule overrides them. That means that even something general like a CSS reset of tables will override any border settings from the ckeditor table plugin.

comment:24 Changed 2 years ago by mencia

19 months have passed since the last post. The situation has been changing. Microsoft have announced they do not support older versions of Internet Explorer after 12/Jan/2016. IE9 is still supported, but it will end on 11/Apr/2017. IE10 is supported too, but the OSes are only 'Windows Server 2012' and 'Windows Embedded 8 Standard'. Do you change any policy about deprecated attributes?

Last edited 2 years ago by mencia (previous) (diff)

comment:26 Changed 2 years ago by mencia

Thank you, niremizov. I'll check the version.

comment:27 Changed 2 years ago by Marek Lewandowski

Priority: NormalNice to have (we want to work on it)

Hi all, thank you all for showing your interest, and providing the feedback on this issue.

We were discussing modernizing table dialog some time ago, however lately we had other parts which we had to focus on.

I'll bump the priority of this issue so that it's more visible for us. We might revisit this during 4.7.x releases as we're looking to enhance tables experience in CKEditor 4.7.0 release.

comment:28 Changed 19 months ago by Marek Lewandowski

For anyone involved in this issue, we're currently working on this problem. Feel free to join the discussion, see https://github.com/ckeditor/ckeditor-dev/issues/619#issuecomment-320922971.

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