Opened 3 years ago

Last modified 21 months ago

#13833 assigned Bug

Styles dropdown doesn't work correctly for tables with border=0

Reported by: Alfonso Martínez de Lizarrondo Owned by: Tomasz Jakut
Priority: Nice to have (we want to work on it) Milestone:
Component: Core : Tables Version: 3.1
Keywords: Cc:

Description

Steps to reproduce

  1. Add some styles to add a class to a table
CKEDITOR.config.allowedContent = true;

CKEDITOR.stylesSet.add( 'my_styles', [
    // Block-level styles
    { name: 'Nice table', element: 'table', attributes: { 'class': 'MyTable' } }
] );

CKEDITOR.config.stylesSet = 'my_styles';
  1. add a little CSS to check that it works
.MyTable {
    border-collapse:collapse;
    border:1px solid blue;
    background-color: #00DD00;
}

.MyTable td, .MyTable th{
    border:1px solid blue;
        padding: 5px 1em;
}

  1. Put some tables in your content with that class and border = 0 or border = 1
  1. Now try to use the Styles dropdown

Expected result

In both tables the Style should be shown as "Nice table"

Actual result

Only works correctly in the second one. The first one has the "cke_show_border" class that CKEditor adds automatically and the Styles system doesn't ignore it. Once you click then it's applied and then it works correctly until you reload the content.

Other details (browser, OS, CKEditor version, installed plugins)

This has been failing for eons, all browsers demo at http://jsfiddle.net/8jcyf9aa/

Change History (19)

comment:1 Changed 3 years ago by Jakub Ś

Status: newconfirmed
Version: 4.5.43.1

Problem can be reproduced from CKEditor 3.1 when "cke_show_border" was introduced.

We could either don't assign this class when table has custom class assigned. But what if custom class doesn't touch borders?

Another approach would be checking if table has border assigned (maybe from getComputedStyle).

comment:2 Changed 3 years ago by Marek Lewandowski

Milestone: CKEditor 4.5.5

There are multiple approaches. The easiest one is to use attribute instead of a class, and this would solve this particular issue. But I'd rather do a quick revise of showborders plugin, as it's pretty simple and I see some IE6 legacy stuff which is no longer needed.

comment:3 Changed 3 years ago by Marek Lewandowski

Milestone: CKEditor 4.5.5CKEditor 4.5.6

comment:4 Changed 3 years ago by Tomasz Jakut

Owner: set to Tomasz Jakut
Status: confirmedassigned

comment:5 Changed 3 years ago by Tomasz Jakut

Status: assignedreview

I've connected getComputedStyle approach with attribute one - so showborders plugin is nearly completely rewritten. I also added some basic tests for it. Pushed branch:t/13833.

comment:6 Changed 3 years ago by Marek Lewandowski

Milestone: CKEditor 4.5.6CKEditor 4.5.7

comment:7 Changed 3 years ago by Marek Lewandowski

Milestone: CKEditor 4.5.7CKEditor 4.5.8

comment:8 Changed 3 years ago by Marek Lewandowski

Milestone: CKEditor 4.5.8CKEditor 4.5.9

comment:9 Changed 3 years ago by Marek Lewandowski

Milestone: CKEditor 4.5.9CKEditor 4.5.10

comment:10 Changed 2 years ago by Marek Lewandowski

Milestone: CKEditor 4.5.10CKEditor 4.5.11

Moving tickets to the next milestone.

comment:11 Changed 2 years ago by Marek Lewandowski

Milestone: CKEditor 4.5.11CKEditor 4.6.1

comment:12 Changed 2 years ago by Marek Lewandowski

Milestone: CKEditor 4.6.1CKEditor 4.6.2

Moving to 4.6.2 minor release, as 4.6.1 is mostly about polishing 4.6.0.

comment:13 Changed 2 years ago by Marek Lewandowski

Just a note: we could extract border function into CKEDITOR.tools.style.parse in a similar fashion as margin method.

comment:14 Changed 2 years ago by Tade0

Status: reviewreview_failed

One thing I noticed is that once the user applies the table style and changes the selection so that it's still within the same cell it used to be, the style dropdown does not react to that.

This happens regardless of the border attribute value.

comment:15 Changed 2 years ago by Tomasz Jakut

Status: review_failedassigned

The issue seems to be more complicated. The whole plugin does not react to any changes in table's styles. If the table has initially borders, then even switching to the borderless styles does not trigger the styles from the plugin.

comment:16 Changed 2 years ago by Tomasz Jakut

Blocked by #16769.

comment:17 Changed 2 years ago by Marek Lewandowski

I've rebased the branch on top of latest master, and added minor doc change.

comment:18 Changed 2 years ago by Marek Lewandowski

Milestone: CKEditor 4.6.2CKEditor 4.7.0

comment:19 Changed 21 months ago by Marek Lewandowski

Milestone: CKEditor 4.7.0
Priority: NormalNice to have (we want to work on it)
Note: See TracTickets for help on using tickets.
© 2003 – 2017 CKSource – Frederico Knabben. All rights reserved. | Terms of use | Privacy policy