Enhanced version of Inline using hook_filter, CCK Imagefield/Filefield, Thickbox, parameters, etc.

Am I correct to assume I should start a new project for this ?

Hello!

Thanks for the reply, I guess I'm just not clear on a few things.

Breaking this code out into individual feature patches could require over 10 seperate patches plus additional "temporary" code along the way just to make each feature patch "stand" on its own. Given the date on your referenced discussion goes back to 2005(!), I get the feeling this this could be a very long, drawn out process.

What has me confused is when you mention:

"For example, the new [nodeinfo] tags wouldn't be supported by me and should be replaced with a new Inline filter syntax, as already mentioned in the other issue and of course in the main issue focussed on this topic."

The [nodeinfo] tag is just the behind the scenes "invisible" glue that is *automatically* added to node fields to allow Inline to act as a true hook_filter based Drupal filter rather then using hook_nodeapi as it is today. (It contains the node ID, field name, and a unique timestamp). They are completely hidden from the user at all times, and are not in any way the new user filter syntax, which matches the pipe delimited format you suggest in the discussion. For example:

[image:2|title=The Sunset|credit=Fred Wilson|align=right]

I did keep in mind existing Inline syntax compatability, and as you'll see it should work with the old syntax as well.

I understand that this makes some very substantial changes to Inline and respect the need for review, testing, feedback, compatibility, etc. That's actually what I was hoping for by offering up the code. However, I honestly have concerns about the amount of time it would take to break this all out and indivually implement piece after piece by committee. I would really rather focus on meeting peoples needs and polishing it up as a whole on a new branch.

Making a big leap forward like this is what 2.x alpha release is all about in my book. 1.x has been frozen for almost 4 months and is always there for people to use if they want something "safe". 2.x doesn't exist at this point. :) So I guess my question is, is this an absolute requirement to spit things up to move ahead? And if not, maybe we could at least open discussion on the changes I listed to see what is not universally agreed upon? I'm more then happy to discuss any part of it individually.

It's great to see code for this issue, that (hopefully) works. However, adding hidden tags is too hacky and certainly will be the cause for other issues in the future. Following this concept, inlining of other nodes or media in other nodes will not be possible, as outlined in the Inline API draft, which is a kind of roadmap / feature-set for the next major release of Inline. But since there is already code for injecting those tags, it should not be that difficult to alter that code to inject |nid=# into existing Inline tags without that parameter. Actually, this is a detachable solution for a specific detail of this issue and I hope that splitting your customized version into separate issues is more understandable now.

I would like to understand this part a bit more if you don't mind, as I really think this is the most important core change in the code.

As you're aware, there isn't any way for hook_filter to get at the NID related to a field it's filtering (largely because filters can filter things other then node fields). From threads I've read on the dev mailing list it doesn't sound like this will change. So if we take the example of inlining images that are attached to the node itself, the filter will need the NID to be supplied in some way. If I'm understanding you correctly, you would just rather see this default NID info inserted into each of the individual tags the user has added (if they didn't supply other NIDs in the tags themselves). Is that correct?

If so, that brings some questions to mind:

• Do you think that simple users (read: not entirely computer savvy :) ) might find this extra default nid=# info injected into each of their inline tags confusing? I'm thinking that since the node itself is sort of the current "default" for any inline included content, that it could just be included via the single nodeinfo tag that's always hidden from users. Otherwise it becomes redundant to inject it repeatedly, and could be confusing for most users who don't care about referencing other nodes. More importantly I really don't see how that would preclude more advanced users from giving a different NID via a tag itself. We would just let it override the one from nodeinfo for that tag.
• What if a webmaster doesn't want to allow their users to be able to Inline content from other nodes? Storing NID in the tags themselves makes this situation pretty clumbsy. Would we inject the default NID into the user's tags, ignore any changes they make to that parameter, and re-populate it with with the original NID on save each time?
• Currently my hidden "nodeinfo" tag has 2 other piece of information. One is the the fieldname, so that specialized filtering can take place for different fields. (ie. a teaser may only have certain inlined image sizes, or maybe an inline video tag shouldn't include the player in the teaser, etc. ) Without this information your limited to filtering the inline tags the same way for every field (teaser, body, cck fields, etc). Much like the previous question, I'm not sure how (or more importantly if) that info would/should injected into the users tags? Unlike NID this isn't something the user should ever be able to overide.
• The third piece of the "nodeinfo" tag is just a simple timestamp so that the content of the node always looks "different" when previewing or saving. Currently Drupal does an hash on the content to decide if it has changed before filtering or saving. If it doesn't see any computed difference in the text then it just shows the cached version when previewing or saving. Inline doesn't have to worry about this right now because it's nodeapi based and gets applied every view. However, using hook_filter this can be a problem. For example, if a user marks his first uploaded image for "deletion", the inlined code for [image=1] needs to change on preview (or saving). The only way to do this is if the content has changed which we are not assured of without this stamp. Again I don't see a way around this without something like a nodeinfo tag?
• Finally, and perhaps most importantly, the issue of the hidden nodeinfo tag causing problems down the line. Is there a senario you have in mind, or are you just thinking that information hidden in a node field will undoubtably cause trouble? :) While I do wish hook_filter had access to the NID and fieldname, I'm not conviced that hiding this small chunk of information in the node is overly dangerous. It's always hidden from the user via nodeapi and would only be visible if the webmaster ditched the Inline module (which would also leave them with lovely user inline tags on everything as well). Even in that case I think a simple uninstall cleanup script could handle removing and previously hidden and visible tags. Basically the functionality of the hidden nodeinfo tag has to come from somewhere for hook_filter to be used. So it really comes down to whether that info is spread across the visible user tags or not. Personally, I think that creates more issues like the ones above, but I'd be interested to hear your perspective further.

I see what you're after with the file#->filename conversion. I guess I figured that the re-ordering of attachements via addition/deletion comes with the territory when choosing to make numerical tag references. From what I remember(!), Inline currently gets tripped up on files "marked for deletion" when previewing though. I don't think it overlooks these marked files when "filtering" tags or changing the # to names. That was part of the reason I decided to do without it originally, as it made dealing with these files cleaner during the preview phase where things can change over and over before you "know" what file the user is trying to reference by number. I'll have to give that some more thought though.

Thanks again for your time!

Thanks! After reading the standards last weekend (oops!) and running some other small modules I made through the Coder module, I figured there would be quite a bit of cleanup for my Inline code. I really wish I would have set tabs to 2 spaces :) I'll apply your patch and run things through Coder to make sure I have a squeaky clean version to work with.

I'm certainly up for better ways to approach Inline using hook_filter. It just becomes a tricky situation as you need to get at information that isn't always directly available. I think once you run through some of the hook_filter and hook_nodeapi code in it you'll see why I chose the route described. I'd be happy to answer any questions you have along the way.

Thanks again..

I agree, the easier for the user, the better. Heck, even let them drag things in via jQuery. :) I've actually have started a JS toolbar module that appears above filtered fields with the intent of inserting inlined images, files, audio, video, as well as teaser breaks, spell checking, etc. At this point it's been set aside but is functionally spellchecking the fields and allowing users to insert teaser breaks. I had only started to flesh out the inline image button, before getting sidetracked looking into some jQuery options.

One interesting thing to note though, is that any tools like this won't have any access to the NID of a node that's just being created. (as that only is known on after save) So there is no way for the tool to fill an initial "|nid=xxx" with the correct info. hook_nodeapi will have to do that dirty work on creation and take care of things (either in all the incomplete initial tags throughout the document, or in creating the nodeinfo tag like you'll see in my code)

Inline API / JavaScript: The node id would be globally available in hook_filter(), but nifty javascripts in f.e. node/#/edit would need extensive and needless workarounds to retrieve such global variables. If a tag depends on certain (not limited to node) data, it should provide it directly. Otherwise, we will end up with a bunch of tags, f.e. [userinfo], [blockinfo], [commentinfo], and so on. Have a look at how Img_Assist module implements neat image support for TinyMCE.

Actually F.E. Javascript tools would never need to get at the current NID if nodeinfo handles it behind the scenes. JS can pull any information on the node being edited directly from the form itself, which it should do so that it's completely current/accurate. Inlined material from other nodes would just have the other NID available in the tag. Other information like user information, block being editied, etc is already available either way per Drual, so I'm not really following you there.

Furthermore, on initial node creation no NID is even known for the JS F.E. to include until the node is sumitted and saved which will create an awkward situation for putting an initial nid in any tags.

Consistency: If adding of files or images from other nodes will be possible someday, we would have tags with those parameters and others without. Furthermore, matching and altering of Inline tags through a database update would be much more complicated.

I think this is where we fundimentally disagree. It's certainly possible to inline material from other nodes. I doubt it would take more then a dozen lines of code in my current module (ignoring all the "security" aspects which would take more code) However IMO inlining information from another NID is just another "optional" paramenter, where as you would like to see an NID in every tag regardless if it's the local node or not. I feel it's optional because:

• I'm willing to bet that 90% of the time people will just be including items that are part of the node they are creating, thus it becomes completely redudant information.
• There will be people (like me) who don't want to let users inline information from other nodes. This would create a very awkward, if not impossible, situation where all the NIDs in the tags would have to be ignored and yet there is no way to get at the local NID in the filter.
• For simple inlining of local node content it's an unecessarily cryptic piece of information that users would see.

I think the "consistancy" agrument is also debateable. Every inlined image has a title also (whether it's the default or not), but we certainly wouldn't include "title=" in every tag.

There wouldn't be any need to reparse inline tags to add nid= to them to achieve adding content from other nodes. It's just an optional paramenter that would overide the curred nid (given the admin wants to allow it and other security constraints are met).

All this aside though, lets look at it another way for a second. Take these other functions that the hidden nodeinfo tag serves which still woudn't be met:

• How will you know the field being filtered (body, teaser, cck_textfield1, etc) in hook_filter without nodeinfo? It can't be included in the tags where users can overide it. Short of searching all the field content in the DB (across multiple tables), it can't be done without passing the field name in.
• How will you force the content to be re-filtered via hook_filter on every submit even if the user doesn't change any text in a field? nodeinfo's timestamp ensures this. Take the example where a user goes in and edits a node by just removing one of the attached images and hits submit. hook_filter won't be run on the content fields as it sees the content hasn't changed (via a hash check). Thus the inline tags won't be refiltered to take into consideration the changes in attachements.

Really I think if you just try re-working it without nodeinfo, you'll end up needed it in the end. I'd love to be wrong, but I don't think there is any magic way around it given how hook_filter works in Drupal.