Opened 10 years ago

Closed 9 years ago

#1752 closed Bug (fixed)

E.tagName has no properties when using tablecommands plugin.

Reported by: Keith Pitt Owned by: Artur Formella
Priority: Normal Milestone: FCKeditor 2.6.4
Component: General Version: FCKeditor 2.4
Keywords: Confirmed Review+ Cc:

Description

You get an error when trying to make text bold in a table when you have the "tablecommands" plugin installed.

E.tagName has no properties

FCKEvents(undefined, undefined) ApplyStyle(Object Element=b _StyleDesc=Object IsCore=true GetType_$=1) FCKCoreStyleCommand() FCKToolbarButton() FCKToolbarButtonUI_OnClick(click clientX=0, clientY=0, Object Name=Bold Label=Bold Tooltip=Bold Style=0 State=0) CancelEvent(click clientX=0, clientY=0)

Attachments (3)

1752.patch (523 bytes) - added by Artur Formella 9 years ago.
1752_2.patch (1.2 KB) - added by Artur Formella 9 years ago.
1752_3.patch (1.1 KB) - added by Artur Formella 9 years ago.

Download all attachments as: .zip

Change History (10)

Changed 9 years ago by Artur Formella

Attachment: 1752.patch added

comment:1 Changed 9 years ago by Artur Formella

Keywords: Confirmed Review? added
Version: FCKeditor 2.5.1FCKeditor 2.4

Steps to reproduce:

  1. In fckconfig.js add:
    	FCKConfig.Plugins.Add( 'tablecommands' );
    

add 'TableHorizontalSplitCell' to default toolbar

2.Paste in source mode:

<p>&nbsp;</p>
<table width="200" cellspacing="1" cellpadding="1" border="1">
    <tbody>
        <tr>
            <td>&nbsp;</td>
            <td>&nbsp;</td>
        </tr>
        <tr>
            <td>&nbsp;</td>
            <td>11111</td>
        </tr>
        <tr>
            <td>&nbsp;</td>
            <td>&nbsp;</td>
        </tr>
    </tbody>
</table>

3.Back to WYSIWIG

4.Click 11111

5.Click Bold. In FF3 (Vista32) you get an error:

oCell.tagName is undefined
http://localhost/edytor/FCKeditor/tags/2.5.1/editor/_source/internals/fcktablehandler_gecko.js
Line 51
51 if ( oCell.tagName.Equals( 'TD', 'TH' ) )
52 aCells[aCells.length] = oCell ;

I attached a simple solution.

comment:2 Changed 9 years ago by Frederico Caldeira Knabben

Keywords: Review- added; Review? removed

The proposed patch fixes the problem, but there a few things to keep attention:

  1. Ok, you are filtering now all ( nodeType == 3 ) nodes... but "if ( oCell.tagName..." is actually looking for ( nodeType == 1 ) stuff. What about potential nodeType that are not 3 or 1? So, instead of filtering the buggy part, the more correct and simple approach is simply going right to the point, by doing "if ( oCell.nodeType == 1 && oCell.tagName...".
  1. The patch should also contain the changelog entry for it.
  1. A small detail... the coding style should follow our standards.

Changed 9 years ago by Artur Formella

Attachment: 1752_2.patch added

comment:3 Changed 9 years ago by Artur Formella

Keywords: Review? added; Review- removed

comment:4 Changed 9 years ago by Alfonso Martínez de Lizarrondo

Keywords: Review- added; Review? removed

wouldn't it work to just use

oCell.nodeName.Equals( 'TD', 'TH' )

?

(I'm asking that question, just searching for a patch even better)

Changed 9 years ago by Artur Formella

Attachment: 1752_3.patch added

comment:5 Changed 9 years ago by Artur Formella

Keywords: Review? added; Review- removed
Owner: set to Artur Formella
Status: newassigned

You're right, it is better solution.

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

Keywords: Review+ added; Review? removed
Milestone: FCKeditor 2.6.4

comment:7 Changed 9 years ago by Artur Formella

Resolution: fixed
Status: assignedclosed

Fixed with [2557]

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