Ticket #3591 (closed Bug: fixed)

Opened 6 years ago

Last modified 5 years ago

Flash url isn't editable after reload

Reported by: alfonsoml Owned by: garry.yao
Priority: High Milestone: CKEditor 3.0
Component: General Version: SVN (CKEditor) - OLD
Keywords: Confirmed Review+ Cc:

Description

Seems to work fine in IE and fails elsewhere:

  1. Load Replace by class sample
  2. Insert a flash movie "abc.swf", set some width and height.
  3. Press OK to close the dialog
  4. Switch to source and back to design.
  5. Open the properties of the flash object.
  6. The url is lost.

Attachments

3591.patch (2.4 KB) - added by garry.yao 6 years ago.
3591_2.patch (1.6 KB) - added by garry.yao 6 years ago.
3591_3.patch (2.3 KB) - added by garry.yao 6 years ago.
3591_4.patch (5.7 KB) - added by garry.yao 5 years ago.
3591_5.patch (6.1 KB) - added by garry.yao 5 years ago.
3591_6.patch (6.5 KB) - added by garry.yao 5 years ago.

Change History

comment:1 Changed 6 years ago by garry.yao

  • Keywords Confirmed added

Changed 6 years ago by garry.yao

comment:2 Changed 6 years ago by garry.yao

  • Owner set to garry.yao
  • Status changed from new to assigned
  • Keywords Review? added

The problem here is that the nested <embed> element which hold the src attribute is not been properly created in the DOM as <cke:embed> due to the <object> filtering, the patch here introducing a hack for allowing CKEDITOR.editor::createFakeParserElement to alternatively filtering the real element contents right before faking it.

Changed 6 years ago by garry.yao

comment:3 Changed 6 years ago by garry.yao

I've thought of a simplified way for this situation:

  1. We'll facing some markups which were produced for cross-browser by our editor, like our current output for swf media:
    <object align="absBottom" classid="clsid:d27cdb6e-ae6d-11cf-96b8-444553540000" codebase="http://download.macromedia.com/pub/shockwave/cabs/flash/swflash.cab#version=6,0,40,0" height="100" width="100"><param name="movie" value="abc.swf" /><embed allowscriptaccess="samedomain" height="100" pluginspage="http://www.macromedia.com/go/getflashplayer" src="abc.swf" type="application/x-shockwave-flash" width="100" wmode="opaque"></embed></object>
    

OR if we need support other alternative output like what from DreamWeaver:

<object classid="clsid:D27CDB6E-AE6D-11cf-96B8-444553540000" width="159" height="91" id="FlashID" title="title">
  <param name="movie" value="../../fckeditor2.6.5/_test/manual/fckdialog/flash.swf" />
  <param name="quality" value="high" />
  <param name="wmode" value="opaque" />
  <param name="swfversion" value="6.0.65.0" />
  <param name="expressinstall" value="Scripts/expressInstall.swf" />
  <!--[if !IE]>-->
  <object type="application/x-shockwave-flash" data="../../fckeditor2.6.5/_test/manual/fckdialog/flash.swf" width="159" height="91">
    <!--<![endif]-->
    <param name="quality" value="high" />
    <param name="wmode" value="opaque" />
    <param name="swfversion" value="6.0.65.0" />
    <param name="expressinstall" value="Scripts/expressInstall.swf" />
    <div>
      <h4>Content of this page require new versoin of Adobe Flash Player。</h4>
      <p><a href="http://www.adobe.com/go/getflashplayer"><img src="http://www.adobe.com/images/shared/download_buttons/get_flash_player.gif" alt="获取 Adobe Flash Player" width="112" height="33" /></a></p>
    </div>
    <!--[if !IE]>-->
  </object>
  <!--<![endif]-->
</object>
  1. As a result of the above reason, we have to eat our own data output of this kind, and we're using the browser to pre-parse those data before they went into the filter pipline, the problem here is the pre-parsing will break the html, by either drop part of the elements or result in malformed markup.
    			// Call the browser to help us fixing a possibly invalid HTML
    			// structure.
    			var div = document.createElement( 'div' );
                            // Actually this will do the opposite, broke the HTML in this case, depend on browsers.
    			div.innerHTML = data;
    
  2. So the idea here is to protect those elements into custom elements which will be treated as what they were by the browser, and those agent elements will be further transforming into fake elements to be used inside wysiwyg mode.

Changed 6 years ago by garry.yao

comment:4 Changed 6 years ago by garry.yao

Custom self-closing tag is not allowed within all none-IE browsers, which will cause incorrect result dom structure, the new patch is about fixing this.

comment:5 Changed 5 years ago by garry.yao

  • Milestone set to CKEditor 3.0

comment:6 Changed 5 years ago by garry.yao

  • Keywords Review- added; Review? removed

Though the patch works, it's now mixing the responsibility of 'htmldataprocessor' with 'flash' plugin, so a better pattern should be found.

Changed 5 years ago by garry.yao

comment:7 Changed 5 years ago by garry.yao

  • Keywords Review? added; Review- removed

Changes containing in this new patch:

  1. flash/plugin.js
    • Protect 'embed/object' into custom elements;
    • Protect '<cke:param />' into '<cke:param></cke:param>';
    • Restore '<cke:param></cke:param>' back into '<cke:param />' with htmlfilter.
  2. htmldataprocessor/plugin.js
    • Scope the htmldataprocessor class to editor instance;
    • Adding 'beforeToHtml' event for pre-processing html support;

comment:8 Changed 5 years ago by garry.yao

  • Priority changed from Normal to High

comment:9 Changed 5 years ago by arczi

Yes, it works, but I'm not sure if we need an event - it should be defined in filter's rules.

This is exactly the same problem like in #3407 - to replace a string using regexp with other string. I think in future someone should create a system solution for all cases.

comment:10 Changed 5 years ago by garry.yao

  • Keywords Review- added; Review? removed

Ok, I'll be waiting for #3407 to see the final solution for protecting elements.

comment:11 Changed 5 years ago by garry.yao

As indicated by Fred, element protecting logic is not plugin specific, e.g. we could also easily have invalid element problem introduced by source mode input, so it's enough to dedicate the protecting logic into htmldataprocessor plugin.
A patch should be coming soon.

Changed 5 years ago by garry.yao

comment:12 Changed 5 years ago by garry.yao

  • Keywords Review? added; Review- removed

comment:13 Changed 5 years ago by fredck

  • Keywords Review- added; Review? removed

I'm ok with most of the changes. Let's just make these regexes simpler (and faster):

var protectElementNamesRegex = /(</?)((?:object|embed|param).*?>)/gi;
var protectSelfClosingRegex = /<cke:param(.*?)\/>/gi;
return html.replace( protectElementNamesRegex, '$1cke:$2');
return html.replace( protectSelfClosingRegex, '<cke:param$1></cke:param>' );

The second regex could be chnaged in the future to accept more than one tag name, but as we can see we don't need it right now.

I didn't test them, but I hope you got the idea.

Changed 5 years ago by garry.yao

comment:14 Changed 5 years ago by garry.yao

  • Keywords Review? added; Review- removed

Tested with all supporting browsers.

comment:15 Changed 5 years ago by fredck

  • Keywords Review+ added; Review? removed

Please change line 206 to the following when committing:

var protectElementNamesRegex = /(<\/?)((?:object|embed|param).*?>)/gi;

comment:16 Changed 5 years ago by garry.yao

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

Fixed with [3779]. Click here for more info about our SVN system.

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