Opened 10 years ago

Closed 10 years ago

Last modified 10 years ago

#7981 closed Bug (fixed)

Iframe dialog not picking width & height values entered in source view

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. Copy an embed code for a video from you tube and paste in to Source view. use the following example:
    <iframe width="560" height="349" src="http://www.youtube.com/embed/39jtNUGgmd4" frameborder="0" allowfullscreen></iframe>
    
  1. Go to Rich text, open context menu on the IFrame & click on Iframe Properties option.

Expected Result: Iframe Properties dialog comes up and shows values of 560px & 349px for width & height fields and also the style field on Advanced tab should show width: 560px; height: 349px.

Actual Result: Iframe Properties dialog comes up but width & height fields are shown empty & also style field on Advanced tab is shown empty.

Attachments (2)

7981.patch (15.6 KB) - added by Garry Yao 10 years ago.
7981_2.patch (7.7 KB) - added by Garry Yao 10 years ago.

Download all attachments as: .zip

Change History (15)

comment:1 Changed 10 years ago by Jakub Ś

Status: newconfirmed

True from rev [6958]

comment:2 Changed 10 years ago by Garry Yao

Owner: set to Garry Yao
Status: confirmedassigned

comment:3 Changed 10 years ago by Satya Minnekanti

Another issue related to this defect is after step 2 when we press OK button with out modifying any values, initial values for width & height are removed and it is restoring IFrame to default height & width

Changed 10 years ago by Garry Yao

Attachment: 7981.patch added

comment:4 Changed 10 years ago by Garry Yao

Status: assignedreview

Propose to revert [6958] to separate styles and attribute-based dimension for now.

comment:5 Changed 10 years ago by Wiktor Walc

I also think we should revert [6958], considering that the it's also not working as expected.

The patch does not seem to be correct though (it contains changes for _source/core/htmlparser/element.js, _source/plugins/fakeobjects/plugin.js and so on)

comment:6 in reply to:  5 Changed 10 years ago by Sa'ar Zac Elias

Status: reviewreview_failed

Replying to wwalc:

I also think we should revert [6958], considering that the it's also not working as expected.

That refers to #7933.

I think reverting the sync is enough, we don't need to bloat the code right now with different stuff (out of the scope, we have #7958 for that).

comment:7 Changed 10 years ago by Wiktor Walc

Milestone: CKEditor 3.6.1

comment:8 Changed 10 years ago by Garry Yao

Just fake object changes are just to make things simpler, but well...we can void them at the moment, besides, I've noticed another regression from [6979] which makes it more than reverting [6958]:

  1. With the ticket tc, but don't open dialog;
  2. Switch to source after load in wysiwyg;
  3. Px units are appended to attribute values

Changed 10 years ago by Garry Yao

Attachment: 7981_2.patch added

comment:9 Changed 10 years ago by Garry Yao

Status: review_failedreview

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

Status: reviewreview_passed

comment:11 Changed 10 years ago by Wiktor Walc

Resolution: fixed
Status: review_passedclosed

Fixed with [7012].

comment:12 Changed 10 years ago by Satya Minnekanti

Even though width & height fields on General Tab showing correct values but the style field on Advanced tab is still empty. Please re open this ticket

comment:13 Changed 10 years ago by Wiktor Walc

@satya - let's discuss it in #7995

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