Skip to content
This repository has been archived by the owner on Jun 26, 2020. It is now read-only.

Commit

Permalink
Merge pull request #127 from ckeditor/t/125
Browse files Browse the repository at this point in the history
Fix: A table cell should always have a `<paragraph>` in the model. Closes #125.

BREAKING CHANGE: The `injectTablePostFixer()` function from  `table/converters/table-post-fixer` is now `injectTableLayoutPostFixer()`and is moved to `table/converters/table-layout-post-fixer` module.
  • Loading branch information
Reinmar authored Oct 2, 2018
2 parents 2f2fe4a + 996136f commit 1eb5d6d
Show file tree
Hide file tree
Showing 5 changed files with 270 additions and 31 deletions.
112 changes: 112 additions & 0 deletions src/converters/table-cell-content-post-fixer.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,112 @@
/**
* @license Copyright (c) 2003-2018, CKSource - Frederico Knabben. All rights reserved.
* For licensing, see LICENSE.md.
*/

/**
* @module table/converters/table-cell-content-post-fixer
*/

/**
* Injects a table cell post-fixer into the model.
*
* The role of the table post-fixer is to ensure that the table cells have the correct content
* after a {@link module:engine/model/model~Model#change `change()`} block was executed.
*
* A table cells must contains at least one block as a child. The empty table cell will have empty `<paragraph>` as a child.
*
* <table>
* <tableRow>
* <tableCell></tableCell>
* </tableRow>
* </table>
*
* Will be fixed to:
*
* <table>
* <tableRow>
* <tableCell><paragraph></paragraph></tableCell>
* </tableRow>
* </table>
*
* @param {module:engine/model/model~Model} model
*/
export default function injectTableCellContentPostFixer( model ) {
model.document.registerPostFixer( writer => tableCellContentsPostFixer( writer, model ) );
}

// The table cell contents post-fixer.
//
// @param {module:engine/model/writer~Writer} writer
// @param {module:engine/model/model~Model} model
function tableCellContentsPostFixer( writer, model ) {
const changes = model.document.differ.getChanges();

let wasFixed = false;

for ( const entry of changes ) {
// Enforce paragraph in tableCell even after other feature remove its contents.
if ( entry.type == 'remove' && entry.position.parent.is( 'tableCell' ) ) {
wasFixed = fixTableCellContent( entry.position.parent, writer ) || wasFixed;
}

// Analyze table cells on insertion.
if ( entry.type == 'insert' ) {
if ( entry.name == 'table' ) {
wasFixed = fixTable( entry.position.nodeAfter, writer ) || wasFixed;
}

if ( entry.name == 'tableRow' ) {
wasFixed = fixTableRow( entry.position.nodeAfter, writer ) || wasFixed;
}

if ( entry.name == 'tableCell' ) {
wasFixed = fixTableCellContent( entry.position.nodeAfter, writer ) || wasFixed;
}
}
}

return wasFixed;
}

// Fixes all table cells in a table.
//
// @param {module:engine/model/element~Element} table
// @param {module:engine/model/writer~Writer} writer
function fixTable( table, writer ) {
let wasFixed = false;

for ( const row of table.getChildren() ) {
wasFixed = fixTableRow( row, writer ) || wasFixed;
}

return wasFixed;
}

// Fixes all table cells in a table row.
//
// @param {module:engine/model/element~Element} tableRow
// @param {module:engine/model/writer~Writer} writer
function fixTableRow( tableRow, writer ) {
let wasFixed = false;

for ( const tableCell of tableRow.getChildren() ) {
wasFixed = fixTableCellContent( tableCell, writer ) || wasFixed;
}

return wasFixed;
}

// Fixes all table cell content by adding paragraph to a table cell without any child.
//
// @param {module:engine/model/element~Element} table
// @param {module:engine/model/writer~Writer} writer
function fixTableCellContent( tableCell, writer ) {
if ( tableCell.childCount == 0 ) {
writer.insertElement( 'paragraph', tableCell );

return true;
}

return false;
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,23 +4,24 @@
*/

/**
* @module table/converters/table-post-fixer
* @module table/converters/table-layout-post-fixer
*/

import Position from '@ckeditor/ckeditor5-engine/src/model/position';
import { createEmptyTableCell, findAncestor, updateNumericAttribute } from './../commands/utils';
import TableWalker from './../tablewalker';

/**
* Injects a table post-fixer into the model.
* Injects a table layout post-fixer into the model.
*
* The role of the table post-fixer is to ensure that the table rows have the correct structure
* The role of the table layout post-fixer is to ensure that the table rows have the correct structure
* after a {@link module:engine/model/model~Model#change `change()`} block was executed.
*
* The correct structure means that:
*
* * All table rows have the same size.
* * None of a table cells that extend vertically beyond their section (either header or body).
* * A table cell has always at least one element as child.
*
* If the table structure is not correct, the post-fixer will automatically correct it in two steps:
*
Expand All @@ -35,12 +36,12 @@ import TableWalker from './../tablewalker';
*
* <table headingRows="1">
* <tableRow>
* <tableCell rowspan="2">FOO</tableCell>
* <tableCell colspan="2">BAR</tableCell>
* <tableCell rowspan="2"><paragraph>FOO</paragraph></tableCell>
* <tableCell colspan="2"><paragraph>BAR</paragraph></tableCell>
* </tableRow>
* <tableRow>
* <tableCell>BAZ</tableCell>
* <tableCell>XYZ</tableCell>
* <tableCell><paragraph>BAZ</paragraph></tableCell>
* <tableCell><paragraph>XYZ</paragraph></tableCell>
* </tableRow>
* </table>
*
Expand All @@ -55,8 +56,8 @@ import TableWalker from './../tablewalker';
* </thead>
* <tbody>
* <tr>
* <td>BAZ<td>
* <td>XYZ<td>
* <td>BAZ</td>
* <td>XYZ</td>
* </tr>
* </tbody>
* </table>
Expand Down Expand Up @@ -90,8 +91,8 @@ import TableWalker from './../tablewalker';
* </thead>
* <tbody>
* <tr>
* <td>BAZ<td>
* <td>XYZ<td>
* <td>BAZ</td>
* <td>XYZ</td>
* </tr>
* </tbody>
* </table>
Expand All @@ -113,8 +114,8 @@ import TableWalker from './../tablewalker';
* <td>12</td>
* </tr>
* <tr>
* <td>21<td>
* <td>22<td>
* <td>21</td>
* <td>22</td>
* </tr>
* </tbody>
* </table>
Expand Down Expand Up @@ -156,8 +157,8 @@ import TableWalker from './../tablewalker';
* <td>(empty, inserted by A)</td>
* </tr>
* <tr>
* <td>21<td>
* <td>22<td>
* <td>21</td>
* <td>22</td>
* <td>(empty, inserted by A)</td>
* </tr>
* <tr>
Expand Down Expand Up @@ -197,32 +198,31 @@ import TableWalker from './../tablewalker';
* <tr>
* <td>11</td>
* <td>12</td>
* <td>(empty, inserted by a post-fixer after undo)<td>
* <td>(empty, inserted by a post-fixer after undo)</td>
* </tr>
* <tr>
* <td>21<td>
* <td>22<td>
* <td>(empty, inserted by a post-fixer after undo)<td>
* <td>21</td>
* <td>22</td>
* <td>(empty, inserted by a post-fixer after undo)</td>
* </tr>
* <tr>
* <td>(empty, inserted by B)<td>
* <td>(empty, inserted by B)<td>
* <td>(empty, inserted by a post-fixer)<td>
* <td>(empty, inserted by B)</td>
* <td>(empty, inserted by B)</td>
* <td>(empty, inserted by a post-fixer)</td>
* </tr>
* </tbody>
* </table>
*
* @param {module:engine/model/model~Model} model
*/
export default function injectTablePostFixer( model ) {
model.document.registerPostFixer( writer => tablePostFixer( writer, model ) );
export default function injectTableLayoutPostFixer( model ) {
model.document.registerPostFixer( writer => tableLayoutPostFixer( writer, model ) );
}

// The table post-fixer.
// The table layout post-fixer.
//
// @param {module:engine/model/writer~Writer} writer
// @param {module:engine/model/model~Model} model
function tablePostFixer( writer, model ) {
function tableLayoutPostFixer( writer, model ) {
const changes = model.document.differ.getChanges();

let wasFixed = false;
Expand All @@ -233,7 +233,6 @@ function tablePostFixer( writer, model ) {
for ( const entry of changes ) {
let table;

// Fix table on table insert.
if ( entry.name == 'table' && entry.type == 'insert' ) {
table = entry.position.nodeAfter;
}
Expand Down
6 changes: 4 additions & 2 deletions src/tableediting.js
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,8 @@ import SetHeaderColumnCommand from './commands/setheadercolumncommand';
import { findAncestor } from './commands/utils';
import TableUtils from '../src/tableutils';

import injectTablePostFixer from './converters/table-post-fixer';
import injectTableLayoutPostFixer from './converters/table-layout-post-fixer';
import injectTableCellContentsPostFixer from './converters/table-cell-content-post-fixer';
import injectTableCellPostFixer from './converters/tablecell-post-fixer';

import '../theme/tableediting.css';
Expand Down Expand Up @@ -145,7 +146,8 @@ export default class TableEditing extends Plugin {
editor.commands.add( 'setTableColumnHeader', new SetHeaderColumnCommand( editor ) );
editor.commands.add( 'setTableRowHeader', new SetHeaderRowCommand( editor ) );

injectTablePostFixer( model );
injectTableLayoutPostFixer( model );
injectTableCellContentsPostFixer( model );

// Handle tab key navigation.
this.editor.keystrokes.set( 'Tab', ( ...args ) => this._handleTabOnSelectedTable( ...args ), { priority: 'low' } );
Expand Down
Loading

0 comments on commit 1eb5d6d

Please sign in to comment.