A conceptual draft.

The amount of feature requests demanding for support of other node types increases. We already have uploads and uploaded image support, and with a patch there is support for attachments, too. In general, we also partially compete against node_attach.module, which performs great for inling other nodes. Then there is img_assist which does a great job for inlining images.

So I thought about a API solution much like the way other modules provide it. Basically we're speaking about inlining of

  1. content attached to a node, f.e. uploads, attachments or audio files
  2. content stored in another node, f.e. videos, audio, stories, pages, ...

Support for each inline content could be supplied through include files, i.e. upload_file.inc, attachment.inc, filestore.inc, audio.inc, node.inc, cck_file.inc and so on. As you can see, this list mixes node attachments with node types of third modules. They should reside in a subdirectory of inline. Each of these plugins has to provide content meta data and theming.

Inline functions, such as automatically inlining uploads as images could be provided through contrib modules, f.e. inline_image.module.

Includes and functions should be addons/plugins, allowing site administrators to de-/activate them through inline settings. Each plugin might add own plugin settings to adjust the output of inline.

The greatest concern regarding this API probably is the notation of inline tags. First of all we might have to weight all activated plugins to handle overlappings between plugins, f.e. applicable to parallely activated upload and attachment module (bad example). Thus, [inline:#] has to be applied to one of both node attachments.

Secondly, inline needs a way to differ between node attachments and other node types. Whilst [node:#] works great for node_attach.module's job, this wouldn't be possible with inline module.

Each plugin might use additional inline API features, f.e. inserting node attachments with a click. A inline_assist.module leveraging this API is also worth thinking about (Obviously this would be based on the great work of img_assist.module and supersede it).

These thoughts incorporate current developments for inline, such as a new annotation for inline tags which is discussed here; advanced theming for uploads here; adding uploads with a click here; adding support for attachments or flash and video; and preventing the use numeric tags here.

It's time for a monster brainstorming.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

sun’s picture

Each plugin has to be able to define its own additional tag attributes. F.e., an audio plugin should be able to handle a tag like [audio:no-player:<file>]. The plugin for image.module in turn would need other tags.

Feature addition raised through audio issue http://drupal.org/node/79955#comment-127557.

sun’s picture

Assigned: Unassigned » sun

Assigning this to me.

sun’s picture

Regarding http://drupal.org/node/73204, http://drupal.org/node/38359 and http://drupal.org/node/32093 inline module should definitely use tag names with a notation of

[inline|nid=#|fid=#]

Since hook_filter() does not supply $node but only $text, this is the only way to get inline working in blocks and whereever else inline is activated as allowed input format. Other plugins for inline would have the ability to add their own custom additions to this tag. This also adds caching for inline tags. As described, img_assist.module already uses this notation of tags.

Without inline_assist and other plugin features (f.e. add uploads by click, http://drupal.org/node/75077) we may implement a transition of old-style [inline:#] tags for preview/prepare. [Strictly speaking of plugin features: This transition is only valid for inlining uploads/attachments and thus, already a plugin feature of those plugins.]

The same transition of old-style to new-style tags also has to be done in a module update, as already mentioned in http://drupal.org/node/38359. This would be a search & replace on all nodes in the database, where [inline:#] gets replaced by [inline|nid=$nid|fid=#].

sun’s picture

  • Disknode module is another candidate which attaches files to nodes like upload or attachment module (http://drupal.org/node/69042).
  • Upload plugin might support a listing of all "inlinable" (i.e. hidden) files by entering [inline:all] which would be transformed to the new scheme [inline|nid=#|fid=all] (http://drupal.org/node/33537).
    • That means: Optional arguments have to be handled by plugins. All inline can do is to provide field validations (f.e. upload's fid might be integer or string and not null, whilst audio's aid has to be an integer). This could be much like the forms API.
sun’s picture

benshell’s picture

I just found this "discussion"... I'm the img_assist maintainer, and I'm definitely planning to do something like this with img_assist (hopefully in time for Drupal 5.0). I have a very rough concept module started where I'm using tags like this:

[inline|iid=123]

Using this method I'm storing all the attributes in a database table, indexed with an "inline ID" (iid). I don't think the NID or FID should be in the tag (or at least I can't think of any reason why it needs to be).

Addon/plugin modules should be able to extend this tag with additional attributes if/when necessary. For example, it'd be nice for a WYSIWYG editor to parse the tag and get an image's width and height (like the img_assist/TinyMCE plugin currently does). Maybe the module could use the node and comment APIs to load in additional attributes at runtime. That way the additional attributes wouldn't have to be stored in the filter tag.

I'd like to add several more attributes to img_assist, but the filter tag is already getting quite long. I'd also like to rename some existing attributes, but I can't really do that on a site that already has img_assist tags. By storing the additional fields in a database table the nodes that use inline API tags will be more future proof.

I'm not sure I agree that plugins should be able to alter the tag name. It seems like using the same syntax, [inline|iid=123] for all inline tags would make life easier. The inline API module would load all the attributes from the database table and then call API functions in the addons/plugins -- just like the node module does when node_load() is called.

What do you think? Sun, have you started coding on any of this? What's your timeframe? I can see that you've also been discussing this for awhile. I've also been thinking about this for some time now, but I'm finally starting to get serious about getting this done.

sun’s picture

Title: inline API » Inline API

Yes, this thing needs a lot of brainstorming and coordination to become a kick-ass-feature. Since my last post is some time ago, let me list up the goals I have in my mind:

  1. API: Create an Inline API for inlining any kind of content, whereas each content type is a plugin that is able to generate its own custom output.
  2. Widgets: Create Inline API widget functions to allow custom widgets (GUIs) for each content type that can be inlined. Widgets may be forms or AJAX (+1 for AJAX). This leads to having just one textarea/RTE button labeled "inline something".
  3. Hooks: Create Inline API hooks to allow certain plugins to interact with Inline (mostly needed for "inline a attached {content} by click").
  4. Code: Make Inline API plugins as easy as possible, like f.e. webform.module components. So any module inlining something may be upgraded to an inline plugin ASAP and new plugins can be written in no time and maintained in Inline API.
  5. Tag: Create a unified tag for Inline API allowing custom arguments for each plugin.

To 1) API:

Mainly generating one grand filter and handles enabled plugins.

  • Provides a settings page to enable/disable plugins, to adjust the processing order of all plugins (by weight) and to control settings of all activated plugins (I'm proposing collabsible fieldsets for each).
  • Handles overall regex matching and provides tag parameter validation.
  • Provides a generic inline widget controller (see 2-Widgets).
  • Outputs tag content by using the corresponding plugin's theme function.
  • Anything that goes through the API has to be cacheable.

Each plugin provides its

  • name (used in tag, e.g. "upload" leads to [inline|upload|...])
  • description and help
  • permission(s)
  • arguments and for each argument a validation (type) and other properties (maxlength f.e.)
  • settings (if any)
  • hook_view() or hook_load() nodeapi functions to allow stuff like inline_auto_add()
  • widget (inline_<plugin>_assist[_validate|_submit|aso.]() functions)
  • hook_menu() if widget is ajaxified
  • theme function that can be overridden

Regarding permission(s): multiple make only sense under rare conditions. F.e. a translator might be restricted to only replace but not insert inlined images, so that would be 'insert image tag' and 'edit image tag', whereas the total count of image plugin tags must not be modified for submitting the translated content. But such permissions could also be managed once for all plugins by Inline API permissions.

Arguments should be defined like the properties for field definitions in Forms API.

Argument validations should be predefined (f.e. int, string, url, aso.) but there should be a validation rule "custom" to allow a totally custom validation for a particular plugin attribute.

No plugin shall duplicate existing functionality. Therefore plugins have to be able to interact with other plugins (f.e. pass a file from [inline|upload|file=example.jpg] to image plugin for rendering/theming).

To 2) Widgets:

img_assist provides a popup currently to select and tag the inlined image. If all that image related stuff is moved to an image plugin of Inline API, we'd have a generic inline widget altogether with an already existing RTE plugin that opens the widget and returns the inline tag into the RTE. I'd love to have just one textarea/RTE button to inline anything a user is permitted to inline. A select box (or maybe neat button graphics?) at the top should load the appropriate plugin widget.

To 3) Hooks:

Although Inline API should provide one central button to insert or edit a tag (that is the current img_assist button), certain plugins want to inject their tags from another location. F.e. file uploads could be inlined by clicking on a button next to the uploaded file in the form. There is no further user interaction needed to place the tag.

To 5) Tag:

You're right, inline tags should uniquely start with [inline|...]. However, storing tag arguments and attributes in a separate datatable is a difficult question.

Pros:

  • cleaner filter tags
  • reference for relations ("Is XY inlined somewhere?")

Cons:

  • translation of inlined content? (i18n)
  • extra datatable requires additional queries
  • inlined content is not editable without widget/GUI
  • different amount of arguments, argument types and argument lengths for each plugin

I hope we'll find a solution somewhere in between that knocks all cons out and gives us the pros, too.
As mentioned in an earlier post the img_assist tag syntax is the right way to go. There are no troubles in argument splitting with URIs, file names, paths or whatever. Although pipe (|) is allowed and would have to be escaped for URLs, this delimiter has the least negative effects.

Last but not least I'd like to mention that much of the code is already done thanks to your excellent img_assist module. However, I'd really like to see inline and img_assist modules merged to Inline API. When all of this will work out, we've to talk to Richard for replacing the inline module and granting CVS access.
As of now I've not coded anything yet since there are still too many questions left open.

Sorry for the big post.

sun’s picture

Forgot the upgrade path: Each plugin might provide an .install file with an system update hook, wherein old-school plugin tags (like [inline:#] or [file:example.jpg=Custom Title]) will be replaced through regular expression matching over the whole database. If we're able to limit that to node.teaser and node.body or perhaps node* datatables (regarding CCK), even better.

aaron’s picture

anyone working on this? a great need, i see. especially as the inline module (as limited as it was) doesn't even work with cck fields.

i've started some work patching the inline module to integrate more cleanly with cck, but don't want to duplicate what's already being worked on.

sun’s picture

I didn't have the time to code something on this yet. But I definitely want to work on the Inline API within the next few weeks.

@benshell: You mentioned that you have a prototype in #6. If this is still so, could you provide that here, so we can join forces? Otherwise I think I'll start from scratch by analyzing img_assist.

@aaron: What do you mean by 'integrate more cleanly with CCK' ? I haven't had any problems regarding CCK yet.

aaron’s picture

If I use CCK, and create a custom theme, calling $node->field[$fieldname][$delta]['view'] directly, the inline 'filter' never gets called, since templating that way ignores teaser/body. I had to call the inline substitution function directly from the node-type.tpl.php file.

I've started working on a more generic solution, and will post something once I figure it out better.

jaharmi’s picture

I'd like to see this replace the functionality I got in the Userland Manila glossary mechanism and the includeMessage macro, but expand upon what was available in that CMS in 1999.

I hope that one could get links/embedding ("http://cnn.com/" or "file.png"), inline display text ("Visit the CNN Web site"), and tooltip text ("CNN").

dww’s picture

I'm highly interested in this. I'm building a newspaper site that's going to make heavy use of img_assist for images. I also want to be able to insert text boxes inside stories, using the same mechanism. Each "insert box" is a new CCK node type with a few custom fields. I want to use inline/img_assist to be able to browse existing insert boxes on the site, or create a new one directly in the pop-up window, and then generate a tag pointing to the insert box node inside the story's body field. Then, when rendering the story node, you see a little insert box inline with the story text...

Shortest-path development would lead me to just fork img_assist, hack it up for my needs, and keep going on solving the rest of the problems on the site. ;) However, that's bad for everyone involved. I'd much rather see Inline API work, and have img_assist (or yeah, inline_assist, even better) be able to handle multiple node types, with a clean mechanism to browse existing nodes for each node type (images == thumbnail gallery, cck nodes == custom view, etc?)

So, what's the story? Is anyone actually working on this now? I don't have a budget, but I sure can code -- however, I don't want to duplicate effort. Moreover, I can't afford to sink 40 hours into a beautiful, general purpose solution if I can get what I need done in 2 hours of hacking img_assist for my own specific purposes. But, I'd be willing to invest, say, 5-10 hours on the general solution, since that'll be better for us in the long run.

Thoughts?

Thanks!
-Derek

sun’s picture

Version: 6.x-2.x-dev » 5.x-2.x-dev

Since my previous follow-ups on this issue I have studied Inline, Image Assist, Wysiwyg, and some other modules thoroughly to get a better idea of Inline API. While I was not yet able to publish that master plan anywhere, I already started to code it. Because this needs to be and will be the next major version of Inline, I committed the initial code to a new DRUPAL-5--2 branch. It is still alpha code and not yet ready for usage on production sites.

I'm currently working on the integration of Inline as a regular input filter, f.e. like img_assist. This has some caveats, though. See http://drupal.org/node/172613 for in-depth discussion about this topic. Opposed to the current state of this discussion, I'm trying hard to find a solution that works without [inline-info] tags.

Previous functionality of Inline has been moved into a separate inline_upload.module, which implements the new hook_inline(). I'm not yet sure if inline_upload.module should rather be a dynamically included file (upload.inc) if Upload module is enabled.
Embedding an attached file inline currently works with a tag like this:

[inline|module=inline_upload|nid=0|file=foo.jpg]
sun’s picture

Category: feature » task

Just committed major improvements for Inline API:

  • Added support for custom inline tag names.
  • Added support for multiple parameter instances (f.e. for gmap).
  • Added support for parameter default values.
  • Fixed invocation for extended parameter validation.
  • Added support for altering macros (f.e. for ex. inline[_upload]).
  • Added support to build/generate macros.
  • Added CCK support.
  • Updated help texts.
  • Changed operation 'args' to Forms API style to support a basic Inline API widget for each Inline implementation.

inline_upload:

  • Fixed node previews.
  • Fixed numeric file references.
  • Re-enabled support for Inline auto-add.
  • Removed obsolete functions from Inline v1.

Any feedback, opinions or reviews are appreciated. Also, thanks to smk-ka, who supplied a patch to make Inline API compatible for gmap.

Mark Theunissen’s picture

Hi sun, I'm going to try your latest build - I'm also interested in this functionality.

Thanks!
Mark

Mark Theunissen’s picture

b.t.w. I found another module with similar goals and functionality, it's called the Drupal Markup Engine. Created by the guys from pingVision.

http://drupal.org/project/dme

sun’s picture

Thanks for this pointer. Unfortunately, DME is quite unusable and looks not very drupalish. However, I somehow like the idea of using HTML-like tags instead of brackets for macros. This has some pros and cons:

+ Macros would probably not be visible, if Inline module is disabled, because browsers do not understand these HTML tags.
+ HTML filter of Drupal core would strip all tags, if Inline module is disabled.
+ An HTML tag attribute-like macro syntax is worth considering, and might help Wysiwyg editors to retrieve attributes from macros more easily.
+ Users are used to HTML-/XML-syntax.

- HTML filter of Drupal core needs a dynamic configuration of allowed tags, because it would strip any Inline tags otherwise.
- No Inline macro could use a tag name that is a HTML or XML reserved word. Compiling an exclusion list of not allowed tags is pretty awkward.
- Macros would become even larger (myfield=whatever compared to myfield="whatever").

Opinions? Heck! We could simply support both!

OT: I've re-considered Benjamin's proposal to store inline tags in the database. As mentioned in http://drupal.org/node/172613#comment-724791, macro rendering functions need to know the field they are acting on. And since we are already updating tags in a node to inject the node id, we can avoid that by simply storing an inline id, object type, node/object id, field name, and tag in the database.
smk-ka proposed to replace macros with [inline|iid=123] upon saving a content, and expanding the macro to the original one upon editing a content. When rendering a content for view, we simply fetch all attributes including internally added ones from the database.

Mark Theunissen’s picture

Hey Sun, I kinda agree about DME, there's no interface for it either, it's just a hook system.

By the way, just in case you're interested, the Flexifilter module (Drupal summer of code project) has appeared now with similar features to Inline... take a look, it's quite good (but only for Drupal 6).

I am frustrated by the number of options available... grr I wish there was just one that everyone used.

Mark Theunissen’s picture

After much more careful consideration and gnashing of teeth, I will probably be using this Inline module, for it looks fantastic!

Thanks,
Mark

sun’s picture

@Mark: I have also reviewed Flexifilter, but it is for a different purpose. Flexifilter allows you (or your users) to create arbitrary custom input filters. These filters are just regular input filters, and are not based on a standardized syntax. That's definitely not what Inline API is about.

jaharmi’s picture

I would also suggest working with the Freelinking module, which provides an input filter for either Wiki-like linking syntax or square bracket links. It handles inter-/intra-site linking; within a Drupal site, double square brackets (the format I use, rather than the Wiki syntax) that contain the name of another node link to that node. You can also change the substitution text, letting you override what text appears within the link (which is very handy so you can match case and so on, making the links fit in and be more natural). It's exceedingly handy.

It’s a little different than embedding content inline, but conceptually as the user I am, they fit together well. There is also precedence for this in another CMS I’ve used, Userland Manila, and its generally unified behavior in handling inline links/includes/data was very powerful and ahead of its time in 1999.

sun’s picture

btw: KarenS and yched are really working hard on the port of CCK for 6.x. Content fields, field formatters, and field widgets have been completely re-factored. This actually opens the opportunity to use existing CCK field formatters and form widgets for Inline API. The current code of Inline API already introduces form widgets for definition of macro parameters. It should be quite easy to integrate CCK field widgets instead. I don't know if re-using of field formatters really makes sense, but I guess we have to find out...

ezra-g’s picture

Subscribing.

setvik’s picture

subscribing

momper’s picture

subscribing

Boris Mann’s picture

coupet’s picture

Good concept. subscribing.

pearcec’s picture

subscribe

rhys’s picture

I've just published a module I've been working on which generically converts any "tag" - i.e. [image], [audio], [video:width=300;height=200] into whatever the module specifies.
It can be found http://drupal.org/project/filter_macros.
Currently it's only d6, and will be backported to d5.
I hope this helps.

dww’s picture

@rhys: I don't see how adding Yet Another Solution(tm) to this problem helps, especially not in a thread talking about trying to unify the problem space, not splinter it further. ;) #333522: How is this different from existing solutions to this problem?

HansBKK’s picture

Encouraged author of new Linodef module (looks interested!) to make contact here - cross-referencing:

http://drupal.org/node/334520

Edit:
Maintainer responded to a question about back-referencing (Q6): "This could be a task for Inline API. I recommend to mention it there."

pfahlr’s picture

I agree about organizing the different inline types into separate plugin modules (inline images, inline flvs, inline pull-quotes etc) since the options will vary substantially. Inline images should have options like which imagecache preset to use, or whether or not to link to the original. While with an flv, you might want to have the option to select a different player. Has anyone considered making use of token module to achieve the text replacement?

pfahlr’s picture

FileSize
1.79 KB

Looking at the development snapshot for 6.x, I think this module could delegate all of the token finding/parsing and replacement theming to plug-in modules for various CCK fields (images, videos, pull quotes) following this general pattern:

function _inline_substitute_tags(&$node, $field) {
  $s = $r = array();
  $tmp = module_invoke_all("inline_substitute_tags", $node, $field);
  $r = $tmp['replace'];
  $s = $tmp['search'];

Plug-in modules would then implement hook_inline_substitute_tags() returning the text to replace and the replacement text. This leaves the plug-in modules to define the syntax between the braces and the generic theme functions used to theme the replacement text. Obviously this would require moving some of the options into the individual plug-in modules. I've begun experimenting with a plug-in for CCK imagefield and it is working for what I need it to do (replace tokens with a thumbnail from imagefield) using the above modification to inline.

Anyone know how to achieve the same as module_invoke_all passing $node by reference? Seems like that would be more efficient.

dww’s picture

"Anyone know how to achieve the same as module_invoke_all passing $node by reference?"

Yes, like so:

  foreach (module_implements('inline_substitute_tags') as $module) {
    $function = $module .'_inline_substitute_tags';
    $function($node, $field);
  }

However, I'm not convinced it's better for each hook implementation to be doing its own parsing of $node->body and replacing things itself. I haven't really thought about it much, just wanted to mention it while I was providing the above code...

pfahlr’s picture

Thanks,

My reasoning for each plug-in parsing $node is that it allows flexibility for each inline attachment type to define its own token syntax. I think the need for this will become more apparent as people want to embed more exotic elements. The demo plug-in module I posted does not modify the node, it returns a list of tokens and suggested replacements.

However, it seems like it's possible to do the parsing within inline module. Maybe like this?

foreach($node->fields as $field) //should be able to enable fields per content type and only loop through supported fields
{
preg_match_all('/\[\[(.*?)\]\]/i', $field->content, $aMatches);
for($i=0;$i<sizeof($aMatches[1]);$i++)
  {
  $aParams = explode(':', $aMatches[1][$i]);
  $aTmp = module_invoke_all('inline_substitute_tags', $aParams);
  //99% sure this mangles the order, but you get the point. 
  $r = array_merge($r, $aTmp);
  }
}
sun’s picture

Version: 5.x-2.x-dev » 6.x-2.x-dev
Status: Active » Needs review
FileSize
18.41 KB

Not that anyone assumes this approach was abandoned... ;)

Attached patch implements the new, rewritten Wysiwyg API, which allows all Drupal modules to interact with any editor. (HEAD)

Specifically, this patch implements support for a [node|nid=123] inline tag on behalf of node.module to display a node inline and exposes a button for editors.

Additionally, the logic was changed so that each module can implement multiple inline tags, i.e. Inline module can implement tags on behalf of Node, Block, User, etc. modules.

Note: Image Assist module will soon depend on this functionality, or, Inline API.

toemaz’s picture

I just tested the 6.x-2.x-dev release on a site where I have been running the 1.x release. Here are my findings:

  • I had to manually enable the inline_upload module in admin/build/modules in order to get the inline filter to work, as it used to work
  • There is no upgrade path yet for converting [inline:filename] to [inline_upload|file=filename]
  • No CCK filefield support yet

I was a bit confused with the last point. I expected the CCK filefield to work since I read that 2.x has CCK support. But soon I realized that CCK support means that the inline filter can be used in text based CCK fields.

I might allocate some time to work on the upgrade path and on CCK filefield support (inline_filefield), during and after DrupalConDC. If anyway has done some work on this already, can you point me to the code?

sun’s picture

- Converting and/or migrating existing inline tags in content is a functionality that does not yet exist in Inline API, but is already on the todo list. Bear in mind though that this needs a solid concept and a very flexible low-level API function. All Drupal modules have to be able to update their inline tags when required. We will need Batch API for that.

- There is no support (and no code) for Filefield yet. Work on this would be appreciated, but I'm also not yet sure whether this will end up in Inline module in the end. My original plan was to ship only support files/modules on behalf of Drupal core modules (like Views does) and let other modules implement the API instead.

toemaz’s picture

I posted a preliminary filefield implementation at http://drupal.org/node/394682

AmrMostafa’s picture

FileSize
1.29 KB

I've applied the patch, tried using the node tag, and it worked. For a real test for API, I thought I would try writing what I actually need for my blog, support for imagecache (from all nodes).

.. and things worked out pretty nicely! :)

Just few random worthless thoughts:

  • Validation should allow storing things in $params. Perhaps a $form_state-like thing could be introduced. $context or $state.
  • Modules should be able to return a validation message explaining the reason. For example, in case of nid to non-existent content. A non-existent imagecache preset, ..etc. Ideally, these should be communicated to the user through drupal_set_message().
  • Is #type used at all (yet)?

Though rather unimportant, the module is attached.

AmrMostafa’s picture

Small concern: In inline_alter_macros(), Is it fundamentally correct to perform a node_save() in nodeapi 'insert'? (i.e. which is like performing a double node_save()s on the same $node, without performing a node_load() in between).

GuillaumeDuveau’s picture

Hi,

I guess Inline 6.x-2.x-dev is supposed to work with WYSIWYG 6.x-2.x-dev but I can't manage to display anything both with FCKEditor and TinyMCE. With TinyMCE I find the option "Inline popups" but there's no button on the GUI, with FCKEditor I can't see any relevant entry in the filter configuration on the WYSIWYG admin page.

I use this on filtered HTML and inline tags is activated in that filter. Teaserbreak plugin from WYSIWYG works with both. Any idea ? Or maybe it's normal and Inline and WYSIWYG are not integrated yet ? Then could the Inline tags be rendered in the WYSIWYG editor like on Sun's mashup and how ? Since it's javascript I guess the only solution is an AJAX query, right ?

Anyway writing manually an Inline tag works, that's already a very good thing :) Sorry if my questions are naive, I'm just discovering WYSIWYG and Inline countries :)

Todd Nienkerk’s picture

I'd like to merge the XML Content Filter module with Inline. It's an API that allows creation of new XML tags as individual modules. These modules define custom attributes, behavior, and output.

sun’s picture

Reviewing my own patch... :)

+++ inline.module	1 Feb 2009 10:35:10 -0000
@@ -14,6 +14,31 @@ function inline_perm() {
+ * Implementation of hook_inline_info().
...
+function inline_inline_info() {

Should contain a @see to point to hook documentation in inline.api.php.

+++ inline.module	1 Feb 2009 10:35:10 -0000
@@ -53,16 +78,48 @@ function inline_filter($op, $delta = 0, 
+ * Invoke a hook_inline() implementation.
+ */
+function inline_invoke($params, $op) {

It's totally unclear who and when and from where this is supposed to be used (or not).

+++ inline.module	1 Feb 2009 10:35:10 -0000
@@ -53,16 +78,48 @@ function inline_filter($op, $delta = 0, 
+  if (!function_exists($params['callback'])) {

Ststic caching would be useful here.

+++ inline.module	1 Feb 2009 10:35:10 -0000
@@ -53,16 +78,48 @@ function inline_filter($op, $delta = 0, 
+      if (!isset($info['callback'])) {
+        $module_tags[$tag]['callback'] = $module . '_' . $tag . '_inline';
+      }

Add a comment to explain that we fall back on an assumed callback here.

+++ inline.module	1 Feb 2009 10:35:10 -0000
@@ -71,11 +128,10 @@ function inline_get_macros($text) {
-    $args = module_invoke($params['module'], 'inline', 'args');
+    $args = inline_invoke($params, 'args');

Now that I read this, and now that $op arguments are (mostly) gone in D7, we should refactor this part accordingly.

+++ inline.node.inc	27 Jan 2009 00:56:51 -0000
@@ -0,0 +1,55 @@
+    case 'validate':
+      // Custom validation of user supplied values.
+      return TRUE;
+

Default value should be TRUE, eliminating this $op for a few implementations.

+++ plugins/dialog.js	27 Jan 2009 00:19:15 -0000
@@ -0,0 +1,19 @@
+Drupal.behaviors.inlineNodeDialog = function (context) {
+  var target = window.opener || window.parent;
+  $('form input[type=submit]', context).click(function () {
+    var settings = {};
+    settings.nid = this.form.nid.value;
+    // Insert into currently attached editor.
+    target.Drupal.wysiwyg.instances[Drupal.settings.instance].insert(target.Drupal.wysiwyg.plugins.inline_node.insert(settings));
+    // Close this dialog.
+    target.Drupal.wysiwyg.instances[Drupal.settings.instance].closeDialog(window);
+    return false;
+  });
+  $('a.form-cancel', context).click(function () {
+    // Close this dialog.
+    target.Drupal.wysiwyg.instances[Drupal.settings.instance].closeDialog(window);
+    return false;
+  });
+}

This entire code needs more docs.

+++ plugins/node.inc	27 Jan 2009 00:20:08 -0000
@@ -0,0 +1,58 @@
+  $plugins = array();

Unnecessary.

+++ plugins/node.inc	27 Jan 2009 00:20:08 -0000
@@ -0,0 +1,58 @@
+function inline_node_wysiwyg_dialog($instance) {
...
+function inline_node_wysiwyg_dialog_form(&$form_state, $instance) {

Is this a hook?

+++ plugins/node.inc	27 Jan 2009 00:20:08 -0000
@@ -0,0 +1,58 @@
+  $output = drupal_get_form('inline_node_wysiwyg_dialog_form', $instance);
+  return $output;

Why 2 lines?

+++ plugins/node.inc	27 Jan 2009 00:20:08 -0000
@@ -0,0 +1,58 @@
+  $form['submit'] = array(
+    '#prefix' => '<div class="container-inline">',
+    '#suffix' => '</div>',
+  );

Wrap into an 'actions' container, please.

+++ plugins/inline_node/inline_node.js	27 Jan 2009 00:16:14 -0000
@@ -0,0 +1,97 @@
+Drupal.wysiwyg.plugins.inline_node = {
+

No blank lines at the beginning or end.

+++ plugins/inline_node/inline_node.js	27 Jan 2009 00:16:14 -0000
@@ -0,0 +1,97 @@
+  // Return whether the passed node belongs to this plugin.
...
+  // Execute the button.
...
+  // Replace inline tags in data.content with images.
...
+  // Replace images with inline tags in editor contents upon data.save.
...
+
+  insert: function (settings) {

JSDoc syntax, please.

+++ plugins/inline_node/inline_node.js	27 Jan 2009 00:16:14 -0000
@@ -0,0 +1,97 @@
+    else {
+      // @todo Plain text support.
+      /*
+    	if (data.content.match(/<!--break-->/)) {
+        return;
+    	}
+    	var content = '<!--break-->';
+      */
+    }
+    if (typeof options != 'undefined') {
...
+    console.log($content);
...
+//      if (this.name != 'mceItemDrupalImage') {
+//        return;
+//      }
...
+    console.log($content);
...
+    return '[node|nid=' + settings.nid + ']';
+    return '<img src="' + settings.path + '/images/spacer.gif" alt="<--break->" title="<--break-->" class="wysiwyg-break" />';

Remove dev code.

Beer-o-mania starts in 23 days! Don't drink and patch.

sun’s picture

Status: Needs review » Active

Committed this patch to HEAD. (6.x-2.x)

Committed the corresponding, required changes to Wysiwyg (3.x).

Reverting status to active. Not sure whether we can mark this fixed already.

sun’s picture

Status: Active » Fixed

Marking as fixed for now.

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.

Vincenzo’s picture

"Inline popups" ships with TinyMCE and it turns regular TinyMCE popup dialogs in overlay javascript popups. Nothing to do with the Inline module, I am afraid.

drzraf’s picture

Can anyone post here the reason this code (in fact, the whole plugins/ directory) is not part of the 7.x branch please ?