Opened 9 years ago

Closed 8 years ago

#922 closed New Feature (fixed)

Generic EMBED and OBJECT placeholder

Reported by: fredck Owned by: martinkou
Priority: Normal Milestone: FCKeditor 2.6
Component: General Version:
Keywords: Confirmed Review- Cc:

Description

Display a generic placeholder for <object> and <embed> tags, similar to the flash placeholder:

  • The default "plugin" icon (a puzzle) should be used for that.
  • It would be nice to have a plugin system which makes it possible to defined object handlers, which are called for each object/embed tag to customize the the placeholder. It would replace the current Flash processor and open the API to easily handle other kinds of objects (like videos, MP3, etc...).
  • A dialog to handle the default attributes for those tags could be also developed, but this is not a requirement for the first implementaiton.
  • It seams that IE "activates" those kinds of objects, and some attributes could be lost because of it. Therefore, these tags should be "protected" (FCK.ProtectTags) before processing (FCKDocumentProcessor).

Change History (9)

comment:1 Changed 9 years ago by w.olchawa

  • Keywords Confirmed added

comment:2 Changed 8 years ago by martinkou

  • Owner set to martinkou
  • Status changed from new to assigned

comment:3 Changed 8 years ago by martinkou

I've opened a branch under https://svn.fckeditor.net/FCKeditor/branches/features/generic_plugin for the generic placeholder code.

I've added a puzzle icon grabbed from http://openclipart.org/media/files/nicubunu/7844, which is public domain. I've found a really nice plugin icon for Tango though (http://www.andreasn.se/blog/?p=17), but it was not officially added to the Tango CVS and so the license terms of that icon is unclear. Fred's suggested puzzle icon from famfamfam was far too small (limited to 16x16) and so I didn't use it.

comment:4 Changed 8 years ago by martinkou

  • Keywords Review? added

The dialog is not implemented yet.

comment:5 Changed 8 years ago by fredck

We've found out that the ProtectSource and ProtectHtml part of the code is not needed, as we can use ProtectTags on IE to solve the <object> problem.

There are also some other improvements that could be done.

  • Looking at the defaultObjectHandler and the FCKFlashHandler code, we see duplication. Maybe it is a good idea having defaultObjectHandler publicly available (FCKEmbedAndObjectProcessor.DefaultObjectHandler), making it possible to reuse that code on custom object handlers.
  • The FCKFlashHandler.Refresh code don't have to check if it is dealing with an <embed> or not. It should be exactly like defaultObjectHandler.Refresh.
  • Actually, the AttachClassIdHandler, AttachFileSuffixHandler and AttachContentTypeHandler should expect a function only to be passed (the current Process function contents), not an object defining the "Process" and "Refresh".
  • The processElement function is to be rewritten. The following should give the exact same results, but is clearer and performs better (not tested):
var processElement = function( el )
{
	// Look for a Content Type processor.
	var type = el.type ;
	if ( type && contentTypeProcessors[type] && contentTypeProcessors[type].Process( el ) )
		return ;

	var nodeName = el.nodeName.toLowerCase() ;

	// Look for a Class ID processor.
	if ( nodeName == 'object' && el.classid )
	{
		var classidProcessor = classidProcessors[ el.classId.replace( /[ \t]/g, '' ).toLowerCase() ] ;

		if ( classidProcessor && classidProcessor.Process( el ) )
			return ;
	}

	// Look for a file name suffix processor.
	if ( nodeName == 'embed' && el.src )
	{
		var suffix = el.src.match( /\.(\w+)(?:\?[0-9A-Za-z!'()*-._~+&=]*)?$/ ) ;
		suffix = suffix && suffix[1] ;

		if ( suffix && suffixProcessors[suffix] && suffixProcessors[suffix].Process( el ) )
			return ;
	}

	// Return the default one.
	defaultObjectHandler.Process( el ) ;
}
  • The exact same changes proposed with the above snippet are applicable to FCKEmbedAndObjectProcessor.RefreshView. That code duplication don't smells good anyway.
  • Came to my eyes that the FCK.IsDirty() save/reset trick done in FCKEmbedAndObjectProcessor.ProcessDocument should be done in FCKDocumentProcessor.Process instead (I know, it was already in the code).

---

After writing the above points and after some long thoughts about it, it seems that our the custom object handler proposal is much more complex than what it should be. It also bring limitations to the system.

Much probably all we really need is: FCKEmbedAndObjectProcessor.AddCustomHandler( <function>). Then for each object or embed found, the custom handler function is called. If it returns "=== false", no further handlers are checked, otherwise the default one is used.

Then, it is up to the custom handler function to test if the element found is good or not for it, by checking the element name, class id, content type, or any other attribute it wants too, making it possible to create custom handlers for <object> elements containing specific <param> elements, for example. It would give much flexibility, making our code changes quite small to provide this generic object placeholder.

It means that, if we go with this simplification, all the above points should be ignored, except the last one.

comment:6 Changed 8 years ago by fredck

  • Keywords Review- added; Review? removed

comment:7 Changed 8 years ago by martinkou

It seems there's no way to keep FCKXhtml from deleting the EMBED attributes in IE5.5. The problem comes from the way IE5.5 manages the attributes collection under elements - it seems to allow only a fixed set of accessible attributes in the attribute collection for each element type. So, when we're converting an HTML source with an <EMBED> to a DOM tree, all the attributes written in the <EMBED> becomes inaccessible from the attributes collection in IE5.5 because we've converted the <EMBED> to an <FCK:EMBED> in the FCK.ProtectTags() function. Subsequently, when we convert the hidden <FCK:EMBED> back to HTML source, all attributes, not just the type attribute, would become lost.

comment:8 Changed 8 years ago by fredck

The IE 5.5 is not a regression. We have the same problem with previous versions, and it seems that no one cares of it. We cannot block the 2.6 because of it.

Martin, please add the changelog entry for it and request review.

comment:9 Changed 8 years ago by martinkou

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

Fixed with [1790].

Click here for more info about our SVN system.

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