Opened 9 years ago

Closed 9 years ago

Last modified 9 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 9 years ago.
7981_2.patch (7.7 KB) - added by Garry Yao 9 years ago.

Download all attachments as: .zip

Change History (15)

comment:1 Changed 9 years ago by Jakub Ś

Status: newconfirmed

True from rev [6958]

comment:2 Changed 9 years ago by Garry Yao

Owner: set to Garry Yao
Status: confirmedassigned

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

Attachment: 7981.patch added

comment:4 Changed 9 years ago by Garry Yao

Status: assignedreview

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

comment:5 Changed 9 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 9 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 9 years ago by Wiktor Walc

Milestone: CKEditor 3.6.1

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

Attachment: 7981_2.patch added

comment:9 Changed 9 years ago by Garry Yao

Status: review_failedreview

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

Status: reviewreview_passed

comment:11 Changed 9 years ago by Wiktor Walc

Resolution: fixed
Status: review_passedclosed

Fixed with [7012].

comment:12 Changed 9 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 9 years ago by Wiktor Walc

@satya - let's discuss it in #7995

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