Opened 7 years ago

Closed 7 years ago

Last modified 6 years ago

#6663 closed Bug (fixed)

Issue with adding carriage returns to Table caption.

Reported by: Satya Minnekanti Owned by: Garry Yao
Priority: Normal Milestone: CKEditor 3.6.1
Component: UI : Dialogs Version: 3.0
Keywords: IBM Cc: Damian, joek, james, c

Description

To reproduce the defect:

  1. Open Ajax sample.
  1. Insert a Table with a caption(Frederico Knabben).
  1. Keep cursor before the word Frederico in the Table caption & press Enter 2 to 3 times
  1. See that the Table is moved down & when you keep the cursor in the Table see that Table orientation is shown in the empty space above & not around the table.
  1. when we check the Table dialog Carriage returns are not shown in the caption field.
  1. Now keep the cursor in the Empty space above the Table and click on Table icon in the Toolbar.
  1. Enter values for Rows and Columns and press OK Button.

Expected Result:

Table is inserted with the entered no of rows & columns.

Actual Result:

Nothing happens and in firebug it is showing the following error.

k.$ is undefined [Break on this error] };d.nodeList.prototype={count:function...Parent().$):j!=i.$&&j.contains(i.$);

Table Caption should not allow Carriage Returns.

Attachments (2)

6663.patch (2.5 KB) - added by Garry Yao 7 years ago.
6663_2.patch (2.1 KB) - added by Garry Yao 7 years ago.

Download all attachments as: .zip

Change History (15)

comment:1 Changed 7 years ago by Garry Yao

Status: newconfirmed
Version: 3.4.23.0

Please don't pack all issues in one ticket on bug reporting, confirmed that line breaks are not supported in table dialog.

comment:2 Changed 7 years ago by Damian

Table captions should support inline content only http://www.w3.org/TR/html401/struct/tables.html#h-11.2.2. If multi-line captions are to be supported they should probably be provided with <br>.

Which milestone can this be targeted for?

Changed 7 years ago by Garry Yao

Attachment: 6663.patch added

comment:3 Changed 7 years ago by Garry Yao

Cc: james c added; James removed
Component: GeneralUI : Dialogs
Owner: set to Garry Yao
Status: confirmedreview

It's hard to propose the right UI to manage inline-contents-filled caption in dialog, while it's always easy to make the editing happen in wysiwyg, so propose for a disabled field in such situation.

comment:4 Changed 7 years ago by Frederico Caldeira Knabben

Status: reviewreview_failed

:/ not really a nice thing to have... it looks like the right fix is a bit more complex.

  • Firstly, we must avoid <p> being created on ENTER inside <caption>, forcing it to <br>.
  • Then, in the caption field in the dialog, we can simply display the innerHTML contents of <caption> in the field (maybe beautified by our simple writer), properly trimmed and without \n. It's an acceptable way for having it, in those few cases where users decide to do manipulations inside <caption>.

comment:5 in reply to:  4 Changed 7 years ago by Garry Yao

Replying to fredck:

it looks like the right fix is a bit more complex.

Not sure it's the right fix, note that apply heading or increase size on caption is quite a normal case, dialog should be just considered as (an less sufficient) compensation for WYSIWYG experience, right now instead u're proposing of treating it just for a comfortable dialog.

comment:6 Changed 7 years ago by Frederico Caldeira Knabben

Well, it looks like we need better ways to justify our opinions here. The issue is much broader now and the right way to go is in fact ambiguous.

HTML4 (and therefore XHTML 1.0 Transitional) says that CAPTION must contain inline stuff only. This justified my previous opinion.

HTML5 instead changed this thing, saying that CAPTION accepts "Flow contents", which includes the basic block elements we're talking about. This change certainly comes from public opinion and makes a lot of sense.

So, better to walk towards the future and accept block elements inside <caption>. Sorry for the precipitated analysis.

In any case, I maintain my R- for the previous patch. The dialog must still maintain the caption editable. It's a valuable feature we don't want to loose.

comment:7 Changed 7 years ago by Garry Yao

Status: review_failedreview

The dialog must still maintain the caption editable. It's a valuable feature we don't want to loose.

6663.patch was implemented exactly in this way (enabled field for text only caption), can do check it again?

Changed 7 years ago by Garry Yao

Attachment: 6663_2.patch added

comment:8 Changed 7 years ago by Garry Yao

Update to [6947].

comment:9 Changed 7 years ago by Frederico Caldeira Knabben

Status: reviewreview_passed

comment:10 Changed 7 years ago by Garry Yao

Resolution: fixed
Status: review_passedclosed

Fixed with [6956].

comment:11 Changed 6 years ago by Frederico Caldeira Knabben

Milestone: CKEditor 3.6.1

comment:12 in reply to:  6 Changed 6 years ago by Damian

The originally reported script error still occurs if the test case steps are followed. If you have the cursor in the caption and you try to create a table in that place, a script error is shown (in firebug). Perhaps in such a case the "create table" dialog shouldn't be available or active?

comment:13 Changed 6 years ago by Garry Yao

I've created #7944 and #7945 for the TCs you described.

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