Ticket #7915 (closed Bug: fixed)

Opened 3 years ago

Last modified 3 years ago

IE: Height value missing from iFrame dialog

Reported by: satya Owned by: garry.yao
Priority: Normal Milestone: CKEditor 3.6.1
Component: General Version: 3.6.1
Keywords: IBM Cc: damo, jamescun, tmonahan

Description

To reproduce the defect:

  1. Open CK Editor and click on IFrame icon.
  1. Enter vales for URL, width(100),height(100) and click OK button.
  1. open Context menu on inserted Iframe and click on IFrame properties option.

Expected Result: IFrame Properties dialog comes up and shows correct values for URL,width & height that we entered in step 2.

Actual Result: IFrame Properties dialog comes up,shows correct values for URL,width that we entered in step 2 but Height value is shown empty.

Attachments

7915.patch (3.8 KB) - added by garry.yao 3 years ago.
7915_2.patch (19.5 KB) - added by garry.yao 3 years ago.
7915_3.patch (21.6 KB) - added by garry.yao 3 years ago.
7915_4.patch (21.8 KB) - added by Saare 3 years ago.
7915_5.patch (27.0 KB) - added by garry.yao 3 years ago.
7915_6.patch (865 bytes) - added by garry.yao 3 years ago.
7915_7.patch (844 bytes) - added by garry.yao 3 years ago.

Change History

comment:1 Changed 3 years ago by j.swiderski

  • Status changed from new to confirmed
  • Version set to 3.6.1 (SVN - trunk)

I was able to reproduce it in IE6, IE7 and IE8. In IE9 it seems to be fine.

This issue has been occurring from rev [6958]

NOTE:If you enter height first and then width then the later one will be missing. So it is about both dimensions and not height only.

comment:2 Changed 3 years ago by fredck

  • Milestone set to CKEditor 3.6.1

Changed 3 years ago by garry.yao

comment:3 Changed 3 years ago by garry.yao

  • Owner set to garry.yao
  • Status changed from confirmed to review

The real problem behind is that, after [6958] iframe dialog use both the attribute and inline style to present dimension, which are conflicting with each other, but why haven't it been aligned with other style-based dialog (like image and table)? Well that's just because a historical reason of fake object which are more convenient to manipulate attributes than with styles.

The proposed patch eliminates attribute based dimension and introduces necessary changes to make this happen easier.

comment:4 Changed 3 years ago by Saare

  • Status changed from review to review_failed

The width/height attributes should be deprecated in the flash dialog as well at the same scope of changing the fake images' attributes based dimensions.

Changed 3 years ago by garry.yao

comment:5 Changed 3 years ago by garry.yao

  • Status changed from review_failed to review

I was trying to avoid a broader range of changes, but it's ok if you think we should get them aligned at one time.

comment:6 Changed 3 years ago by Saare

  • Status changed from review to review_failed

The ticket TC fails with the last patch.

comment:7 Changed 3 years ago by Saare

Latest patch fixes the original TC and contains minor improvements.

Changed 3 years ago by garry.yao

comment:8 Changed 3 years ago by garry.yao

  • Status changed from review_failed to review

comment:9 Changed 3 years ago by Saare

  • Status changed from review to review_failed

Insert the following embed-less object:

<object width="251" height="200" data="http://releases.flowplayer.org/swf/flowplayer-3.2.5.swf" type="application/x-shockwave-flash">
	<param name="movie" value="http://releases.flowplayer.org/swf/flowplayer-3.2.5.swf" />
	<param name="allowfullscreen" value="true" />
	<param name="allowscriptaccess" value="always" />
</object> 
  1. Switch back to source. An empty style attribute is being added.
  2. Open the dialog with the context menu. The width and height attributes are not loaded.
  3. Set some width/height and click OK. JS error is thrown.

Changed 3 years ago by Saare

comment:10 Changed 3 years ago by Saare

The latest patch fixes those problems, along with some code style fixes and KISS opportunities.

comment:11 Changed 3 years ago by garry.yao

Oops, back-compact is broken by my previous patch, but I'm afraid that force convertion wouldn't be an option, as it mangles user's content on data output, let's temporarily follow the current conversion in image dialog - attributes are dropped only if the dimension is modified by dialogs. I'll come with a new patch.

Changed 3 years ago by garry.yao

comment:12 Changed 3 years ago by garry.yao

  • Status changed from review_failed to review

comment:13 Changed 3 years ago by Saare

  • Status changed from review to review_failed
  • IE: Ticket TC is not resolved.
  • Insert an iframe, leaving the width/height fields blank. Result: fake image is sized 1*1.
  • With the contents in comment:9, open the dialog and go to the advanced tab. In the styles field, put "width:80px". Switch to general tab and see that the height field disappears.

Also.. ther're some coding style mistakes and KISS opportunities, e.g. the changes in dialogadvtab and the new cssStyle parser. Please review the code changes.

Last edited 3 years ago by Saare (previous) (diff)

Changed 3 years ago by garry.yao

comment:14 Changed 3 years ago by garry.yao

  • Status changed from review_failed to review

As more problems are found with the patch, I'd fallback the patch to only tackle the original issue of this ticket and conduct the continuation at #7958.

comment:15 Changed 3 years ago by Saare

  • Status changed from review to review_failed

The event should not be fired if the field hasn't changed.

Changed 3 years ago by garry.yao

comment:16 Changed 3 years ago by garry.yao

  • Status changed from review_failed to review

comment:17 Changed 3 years ago by Saare

  • Status changed from review to review_passed

comment:18 Changed 3 years ago by garry.yao

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

Fixed with [7001].

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