Ticket #171 (review_failed Bug)

Opened 7 years ago

Last modified 20 months ago

Unable to set textfield char width to 20

Reported by: henryli86@… Owned by: martinkou
Priority: Low Milestone:
Component: General Version: 3.0
Keywords: IE6 IE7 Cc: alfonsoml

Description

Hi All,

I had trouble entering "20" to Character Width of Text Field and Columns of TextArea. It simply got missing. I do not know if this is intentionally or a bug.

Your help is highly appreciated,

Henry


Moved from SF:
http://sourceforge.net/tracker/index.php?func=detail&aid=1661543&group_id=75348&atid=543653

Attachments

171_pre.patch (3.7 KB) - added by martinkou 7 years ago.
171.patch (5.5 KB) - added by martinkou 7 years ago.
171_2.patch (6.4 KB) - added by martinkou 7 years ago.

Change History

comment:1 Changed 7 years ago by fredck

  • Cc alfonsoml added
  • Keywords Confirmed added
  • Reporter changed from fredck to henryli86@…

That's the default value for IE and it doesn't seem to provide that value back when it's set. So yes, it is a bug but in IE and at first look it isn't easy to know when it has been set to 20 or it isn't present in the original html.


Moved from SF. Original poster: alfonsoml

comment:2 Changed 7 years ago by fredck

  • Milestone set to FCKeditor 2.6

comment:3 Changed 7 years ago by martinkou

  • Owner set to martinkou
  • Status changed from new to assigned

comment:4 Changed 7 years ago by martinkou

The protected attribute feature proposed in the patch to #617 should be useful for fixing this issue. I need to wait for the #617 patch's review to complete before fixing this.

comment:5 Changed 7 years ago by martinkou

I've done some investigations today, the nature of this ticket is similar to #617 and so the extent of changes required would be pretty large.

The issue originates from IE itself. Say, you have an <input type="text"> DOM node, and you assign a size="20" attribute to it in JavaScript, then the input node's HTML code would not be changed. i.e. the size="20" would not appear in document.body.innerHTML at all. The attribute would also appear to be undefined when you try to examine it in JavaScript via DOM methods. These cause problems when the user tries to examine the text box again by opening the text field dialog box, or examine the HTML code in source mode.

But unlike #617, there's one more way for this IE bug to present itself for this ticket. When you set the innerHTML of any element (say, document.body) with an <input type="text" size="20">, IE would still automatically ignore the size="20" attribute it got from the HTML code. So, even if we've got the DOM->HTML part right (i.e. the user can see size="20" in Source mode), the attribute would still disappear if the user presses the Source button again and see the text box in WYSIWYG mode.

If a fix were to be implemented, then at least the following have to be changed:

  1. The text box dialog - it must read and write to an alternative size attribute since you can't set the value to 20 in the real size attribute.
  2. FCKXHtml - it must be able to somehow see the alternative size attribute and convert it to a real size attribute in the HTML string output, when the user switches from WYSIWYG to Source mode or when the user submits the HTML back to the server.
  3. FCK.SwitchEditMode() - it must be able to convert any <input type="text" size="20"> to the alternative size attribute when switching from Source mode to WYSIWYG mode since setting size="20" in HTML code is useless.

The patch in #617 has only implemented point 2 and the changes were already deemed too extensive for 2.6. So maybe this ticket should be left for 2.7 as well?

comment:6 Changed 7 years ago by martinkou

I've actually implemented point 1 and 2 today, but point 3 isn't implemented yet. The current patch should be able to serve as a reference even if the ticket is retargeted to 2.7.

Changed 7 years ago by martinkou

Changed 7 years ago by martinkou

comment:7 Changed 7 years ago by martinkou

  • Keywords Review? added

After some discussion with Fred, it was decided the issue should be fixed in 2.6. I've made a patch which expands on the href protection logic of A tags.

Changed 7 years ago by martinkou

comment:8 Changed 7 years ago by martinkou

Updated the patch since the previous one would result in JavaScript errors in the Paste from Word dialog.

comment:9 Changed 7 years ago by alfonsoml

  • Keywords Review- added; Review? removed

The problem that I see is that it does add even more preprocessing time to cover just a very specific problem. And there are other issues that should be added if we are trying to protect all the IE problems. For example you have protected the textarea:cols, but I'm sure that there's a default value for rows that will make it also disappear.

I remember that in another bug the problem was the margin properties of the body, in that case the values were something like 15 10 15 10 or the like

If someone really cares about the dimensions then he will be using styles, and after all if the problem is present only when the property has its default value I don't see a big problem with it. I mean: not big enough as to add more time, there are people complaining that loading big HTML files in the editor is slow, every regexp that we add can make the problem even worse.

Also, this is an IE only issue, so at the very least, those regexp aren't needed for the rest of browsers.

I think that if this patch is added it should be applied only in the IE code.

comment:10 Changed 7 years ago by fredck

  • Milestone changed from FCKeditor 2.6 to FCKeditor 3.0

I agree with Alfonso about two things: this is not an important bug and it should be applied to IE only.

For the V3, all forms features will be separated in a specific plugin. At that point, we will be able to provide fixes like this together with the plugin, so it will impact only those who really need it. So, let's delay it for that.

In any case, the proposed solution is correct, and it is most probably supposed to be the one to be used.

comment:11 Changed 5 years ago by fredck

  • Milestone changed from CKEditor 3.0 to CKEditor 3.x

comment:12 Changed 4 years ago by fredck

  • Milestone CKEditor 3.x deleted

Milestone CKEditor 3.x deleted

comment:13 Changed 23 months ago by j.swiderski

  • Keywords SF Confirmed removed
  • Version changed from FCKeditor 2.4 to 3.0

This is still an issue in latest CKEDitor 3.6.4 qand IE6-7 (IE8 and IE9 work fine)

comment:14 Changed 20 months ago by j.swiderski

  • Priority changed from Normal to Low
  • Keywords IE6 IE7 added

This is still an issue in CKEditor 4.x (v4).

To reproduce:

  1. Just insert textfiled using toolbar button.
  2. In textfield dialog just enter 20 in width field.

When you switch to source you will notice that there is no size attribute. Please note that this is only in IE7 and only with value 20.

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