Ticket #4246 (closed Bug: fixed)

Opened 5 years ago

Last modified 4 years ago

Avoid deprecated image attributes

Reported by: mike890 Owned by: garry.yao
Priority: Normal Milestone: CKEditor 3.1
Component: Core : Output Data Version: 3.0 RC
Keywords: IBM Confirmed Review+ Cc: jkavanag@…, mimo@…

Description

The latest build of FCK 3.0 still generates the deprecated image attributes as align/hspace/vspace. All these attributes has been deprecated for several years already for the xhtml strict mode. They are also deprecated in HTML5.

Maybe the new version of the editor would be the great time to switch to the styles instead of these attributes? Otherwise, the generated HTML cannot pass validation.

Attachments

fck_image.js (15.1 KB) - added by Schmankerl 4 years ago.
4246.patch (16.0 KB) - added by garry.yao 4 years ago.
4246_2.patch (21.4 KB) - added by garry.yao 4 years ago.
4246_3.patch (23.8 KB) - added by garry.yao 4 years ago.
4246_4.patch (23.5 KB) - added by garry.yao 4 years ago.
4246_5.patch (24.2 KB) - added by garry.yao 4 years ago.
4246_6.patch (24.8 KB) - added by garry.yao 4 years ago.

Change History

comment:1 Changed 5 years ago by garry.yao

  • Keywords Confirmed added
  • Component changed from General to Core : Output Data
  • Milestone set to CKEditor 3.1

comment:2 Changed 5 years ago by WALoeIII

+1

This is very frustrating since we use a comprehensive reset stylesheet that removes all margins and padding to ensure we have full control.

comment:3 Changed 5 years ago by fredck

  • Summary changed from FCK 3.0 generates deprecated image attributes to Avoid deprecated image attributes

We must just be sure the editor will remain backwards compatible with content produced in the current way.

Changed 4 years ago by Schmankerl

comment:4 Changed 4 years ago by Schmankerl

Hello

I have written a workaround for using css instead of img tags, but has some restrictions. Please take a look and correct the code.

Important!!!:

  • It replaces old hspace, vspace, align automatically.
  • You can`t use stylefield in advanced tab any more.

comment:5 Changed 4 years ago by garry.yao

#4631 has been marked as DUP.

comment:6 Changed 4 years ago by fredck

Just for reference, #4631 also included the border handling into the list of things to be changed by this ticket.

comment:7 Changed 4 years ago by JoeK

  • Keywords IBM added
  • Cc jkavanag@… added

comment:8 Changed 4 years ago by garry.yao

  • Owner set to garry.yao
  • Status changed from new to assigned

Changed 4 years ago by garry.yao

comment:9 Changed 4 years ago by garry.yao

  • Keywords Review? added

comment:10 Changed 4 years ago by fredck

  • Keywords Review- added; Review? removed
  • The border doesn't get removed by simply emptying the border field.
  • We should not have two separated options "Align" and "Float". For the eyes of end users, we're always talking about "Align". So, for now, we could reduce the number of alignment options to "Left" and "Right" only. When loading an image with the "align" attribute, we should remove it only if it matches "left" or "right", otherwise we just leave it intact. No vertical alignment should be considered at this moment.
  • Actually... it looks like changes are not allowed at all, once you set some styles in the image.

comment:11 Changed 4 years ago by garry.yao

Provide the spec behind the idea of only looking at the short-hand (composed) rule to determinate the inline style is here:

http://www.w3.org/TR/DOM-Level-2-Style/css.html#CSS-CSS2Properties

Changed 4 years ago by garry.yao

comment:12 Changed 4 years ago by garry.yao

  • Keywords Review? added; Review- removed

Fixing the above mentioned issues.

comment:13 Changed 4 years ago by fredck

  • Keywords Review- added; Review? removed
  • If you set the border or alignment and confirm and then editing it you remove the previously set value, an empty "style" attribute remains in the source.
  • The border style has a space before the semicolon (e.g. "border: 1px solid ;" instead of "border: 1px solid;". That's just a small detail, of course.
  • The dialog is broken when inserting new images. It works well for existing images only.
  • In IE, when editing an image with src only, when loading the dialog the HSpace value is "-1". By cleaning this field and confirming the dialog, "-1" margins get added to the image.

I'm not adding other issues here as it looks like we already have several listed. I think the next patching round must be tested better before putting it under review.

Changed 4 years ago by garry.yao

comment:14 Changed 4 years ago by garry.yao

  • Keywords Review? added; Review- removed

Ticket TCs updated.

comment:15 Changed 4 years ago by fredck

  • Keywords Review- added; Review? removed

It's working as a charm in Firefox. Still issues in IE though:

  • It's not possible to add borders when editing an image that previously had no border.
  • It's not possible to remove the borders of images with borders.

Changed 4 years ago by garry.yao

comment:16 Changed 4 years ago by garry.yao

  • Keywords Review? added; Review- removed

The culprit in IE is just onChange is not fired when typing, instead we have 'onKeyUp' now.

comment:17 Changed 4 years ago by garry.yao

The border style has a space before the semicolon (e.g. "border: 1px solid ;" instead of "border: 1px solid;" in Firefox.

A browser issue for me, Firefox is expecting the third component of 'border' short-hand in position, which leave there an empty space.

comment:18 Changed 4 years ago by fredck

  • Keywords Review- added; Review? removed

With IE:

  1. Load the following HTML:
<p><img alt="" src="http://www.google.com/intl/en_ALL/images/logo.gif" style="border-bottom: 5px solid; border-left: 5px solid; border-top: 5px solid; border-right: 5px solid" title="" /></p>
  1. Open the dialog for the image.
  1. Click the "Reset size" button.
  1. Confirm the dialog.

The sizes will not be in the output.

Changed 4 years ago by garry.yao

comment:19 Changed 4 years ago by garry.yao

  • Keywords Review? added; Review- removed

comment:20 Changed 4 years ago by fredck

  • Keywords Review- added; Review? removed

With IE:

  1. Load the following HTML:
<p><img alt="" src="http://www.google.com/intl/en_ALL/images/logo.gif" style="width: 276px; height: 110px" title="" /></p>
  1. Open the dialog for the image.
  1. Set the width to "100" the height will be automatically set to "40".
  1. Confirm the dialog.

The height will not change.

Changed 4 years ago by garry.yao

comment:21 Changed 4 years ago by garry.yao

  • Keywords Review? added; Review- removed

comment:22 Changed 4 years ago by fredck

  • Keywords Review+ added; Review? removed

comment:23 Changed 4 years ago by garry.yao

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

Fixed with [4685] and [4686].

comment:24 Changed 4 years ago by miiimooo

  • Cc mimo@… added
  • Resolution fixed deleted
  • Version changed from 3.0 RC to 3.2
  • Type changed from Bug to New Feature
  • Status changed from closed to reopened

I can confirm that the new version fixes the align= problem. Would it be possible/difficult to add support for replacing the style= with a CSS class for more consistent design (similar to the justifyClasses and indentClasses)? Maybe call it imageAlignClasses...

comment:25 Changed 4 years ago by fredck

  • Status changed from reopened to closed
  • Resolution set to fixed
  • Version changed from 3.2 to 3.0 RC
  • Type changed from New Feature to Bug

Please do no reopen tickets for feature requests.

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