Ticket #6663 (closed Bug: fixed)

Opened 4 years ago

Last modified 3 years ago

Issue with adding carriage returns to Table caption.

Reported by: satya Owned by: garry.yao
Priority: Normal Milestone: CKEditor 3.6.1
Component: UI : Dialogs Version: 3.0
Keywords: IBM Cc: damo, 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

6663.patch (2.5 KB) - added by garry.yao 3 years ago.
6663_2.patch (2.1 KB) - added by garry.yao 3 years ago.

Change History

comment:1 Changed 4 years ago by garry.yao

  • Status changed from new to confirmed
  • Version changed from 3.4.2 to 3.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 4 years ago by damo

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 3 years ago by garry.yao

comment:3 Changed 3 years ago by garry.yao

  • Status changed from confirmed to review
  • Cc james, c added; james c removed
  • Component changed from General to UI : Dialogs
  • Owner set to garry.yao

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 follow-up: ↓ 5 Changed 3 years ago by fredck

  • Status changed from review to review_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 3 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 follow-up: ↓ 12 Changed 3 years ago by fredck

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 3 years ago by garry.yao

  • Status changed from review_failed to review

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 3 years ago by garry.yao

comment:8 Changed 3 years ago by garry.yao

Update to [6947].

comment:9 Changed 3 years ago by fredck

  • Status changed from review to review_passed

comment:10 Changed 3 years ago by garry.yao

  • Status changed from review_passed to closed
  • Resolution set to fixed

Fixed with [6956].

comment:11 Changed 3 years ago by fredck

  • Milestone set to CKEditor 3.6.1

comment:12 in reply to: ↑ 6 Changed 3 years ago by damo

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 3 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 – 2012 CKSource – Frederico Knabben. All rights reserved. | Terms of use | Privacy policy