Opened 13 years ago

Closed 13 years ago

#7915 closed Bug (fixed)

IE: Height value missing from iFrame dialog

Reported by: Satya Minnekanti Owned by: Garry Yao
Priority: Normal Milestone: CKEditor 3.6.1
Component: General Version: 3.6.1
Keywords: IBM Cc: Damian, James Cunningham, Teresa Monahan

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 (7)

7915.patch (3.8 KB) - added by Garry Yao 13 years ago.
7915_2.patch (19.5 KB) - added by Garry Yao 13 years ago.
7915_3.patch (21.6 KB) - added by Garry Yao 13 years ago.
7915_4.patch (21.8 KB) - added by Sa'ar Zac Elias 13 years ago.
7915_5.patch (27.0 KB) - added by Garry Yao 13 years ago.
7915_6.patch (865 bytes) - added by Garry Yao 13 years ago.
7915_7.patch (844 bytes) - added by Garry Yao 13 years ago.

Download all attachments as: .zip

Change History (25)

comment:1 Changed 13 years ago by Jakub Ś

Status: newconfirmed
Version: 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 13 years ago by Frederico Caldeira Knabben

Milestone: CKEditor 3.6.1

Changed 13 years ago by Garry Yao

Attachment: 7915.patch added

comment:3 Changed 13 years ago by Garry Yao

Owner: set to Garry Yao
Status: confirmedreview

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 13 years ago by Sa'ar Zac Elias

Status: reviewreview_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 13 years ago by Garry Yao

Attachment: 7915_2.patch added

comment:5 Changed 13 years ago by Garry Yao

Status: review_failedreview

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 13 years ago by Sa'ar Zac Elias

Status: reviewreview_failed

The ticket TC fails with the last patch.

comment:7 Changed 13 years ago by Sa'ar Zac Elias

Latest patch fixes the original TC and contains minor improvements.

Changed 13 years ago by Garry Yao

Attachment: 7915_3.patch added

comment:8 Changed 13 years ago by Garry Yao

Status: review_failedreview

comment:9 Changed 13 years ago by Sa'ar Zac Elias

Status: reviewreview_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 13 years ago by Sa'ar Zac Elias

Attachment: 7915_4.patch added

comment:10 Changed 13 years ago by Sa'ar Zac Elias

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

comment:11 Changed 13 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 13 years ago by Garry Yao

Attachment: 7915_5.patch added

comment:12 Changed 13 years ago by Garry Yao

Status: review_failedreview

comment:13 Changed 13 years ago by Sa'ar Zac Elias

Status: reviewreview_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 13 years ago by Sa'ar Zac Elias (previous) (diff)

Changed 13 years ago by Garry Yao

Attachment: 7915_6.patch added

comment:14 Changed 13 years ago by Garry Yao

Status: review_failedreview

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 13 years ago by Sa'ar Zac Elias

Status: reviewreview_failed

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

Changed 13 years ago by Garry Yao

Attachment: 7915_7.patch added

comment:16 Changed 13 years ago by Garry Yao

Status: review_failedreview

comment:17 Changed 13 years ago by Sa'ar Zac Elias

Status: reviewreview_passed

comment:18 Changed 13 years ago by Garry Yao

Resolution: fixed
Status: review_passedclosed

Fixed with [7001].

Note: See TracTickets for help on using tickets.
© 2003 – 2022, CKSource sp. z o.o. sp.k. All rights reserved. | Terms of use | Privacy policy