[2.0] Howto properly generate serendipity_editor.js from tpl

[Timbalu]

I still have that issue about moving the 2.0/2k11/config.inc.php serendipity_plugin_api_pre_event_hook events into core, or choose a better approach.

There are some things to happen with the current implementation.

What if anyone has its own template with a config.inc and a pre_event hook? In that case the backend and frontend is missing all events that happened to be defined in the $serendipity['defaultTemplate'].
Another thing to happen is, since the $serendipity['defaultTemplate'] config.inc will be loaded anyway(?), that we will have a possible function 'redeclare' issue here.

Therefore - to make this "core functionality" more independent - I would like to have these events (*) happen as some $serendipity['core_events'] in plugin_api.inc, somewhere around https://github.com/s9y/Serendipity/blob ... .php#L1050 (concrete ideas welcome).
What do you think about this suggestion?

(*) without using a wrapping pre event function declaration, to avoid possible "Fatal error: Cannot redeclare serendipity_plugin_api_pre_event_hook() previously declared in" errors.

----- -----
PS. This post was moved by onli, without asking me. I am not happy with that! (I would never do that with dev colleagues!)
Though I don't want to play silly games, I therefore leave it as is. But I still see that related to the "2.0 wysiwyg" topic discussion, particularly since there are still some related posts residing and my last changes to these events are strictly based to the JS-Editor. So I don't see the need to move this and give it this title "[2.0] Howto properly generate serendipity_editor.js from tpl", since that is not my actual point. I want to discuss, moving all events currently fired in that function to my suggested position, or even better. (Btw, moving forum posts destroys links)

[onli]

Another thing to happen is, since the $serendipity['defaultTemplate'] config.inc will be loaded anyway(?), that we will have a possible function 'redeclare' issue here.

That is not how it works. If I'm not very mistaken, the config of $serendipity['defaultTemplate'] is completely ignored. That is why default has a static clone of serendipity_editor.js.tpl, namely /templates/default/admin/serendipity_editor.js, to which every plugin which is not 2k11 can fall back to.

Therefore - to make this "core functionality" more independent - I would like to have these events (*) happen as some $serendipity['core_events'] in plugin_api.inc, somewhere around https://github.com/s9y/Serendipity/blob ... .php#L1050

I wouldn't relate this to plugins at all.

First, let's look at the two used hooks. One is the "js"-hook emitting a piece of code for the frontend. In my book, it is perfectly fine to have that in the config.inc.php, and the alternative is to simply put it into a js-file and link it manually in the relevant entries.tpl.

The other one is serendipity_editor.js.tpl. What we might want here instead of using the event-hook is a mechanism which allows the template to simply link to a admin/serendipity_editor.js, and if that does not exist but it exists the admin/serendipity_editor.js.tpl, this template is parsed and the generated js-code echoed. That could go through the core.

[Timbalu]

That is not how it works. If I'm not very mistaken, the config of $serendipity['defaultTemplate'] is completely ignored. That is why default has a static clone of serendipity_editor.js.tpl, namely /templates/default/admin/serendipity_editor.js, to which every plugin which is not 2k11 can fall back to.

That is why I wrote "anyway(?)". No. That is not the case for the latter!
https://github.com/s9y/Serendipity/blob ... #L124[code]/* Name of folder for the default theme */
$serendipity['defaultTemplate'] = '2k11';
[/code]
This ist not "templates/default" ! In case of a user choosen $serendipity['template'] there is no fallback to precompiled default/serendipity_editor.js, IMO. Did you test this?

One is the "js"-hook emitting a piece of code for the frontend.

As I noted in my commits, this new hook is for both: backend and frontend. It is also used by plugins to populate the serendipity.js with some otherwise thrown inline javascript, see new freetag for example. That makes it independent and being a core thing.
Yes, the jquery code you put in there, seems related to the 2k11 frontend. If it has to be there only, give it a better access.

If you install an other theme w/o backend, the files serendipitry_editor.js and serendipity.js are completely empty. I don't think this was intended to have. And that issue is related to the position of these more or like core events.

[onli]

This ist not "templates/default" ! In case of a user choosen $serendipity['template'] there is no fallback to precompiled default/serendipity_editor.js, IMO. Did you test this?

In fact I'm pretty sure I did, though it has been a while. There is another fallback line after $serendipity['defaultTemplate'], which harcoded falls back to templates/default. That worked, and serendipity_editor.js was definitely not empty. If it doesn't work anymore, something else broke that - and we just need to make sure that serendipity_getFile in smarty finds the hardcompiled js-file to have at least the current system working.

It is also used by plugins to populate the serendipity.js with some otherwise thrown inline javascript, see new freetag for example

I think you are mixing up stuff here, or I don't understand you properly. The js-hook is in fact generated in the core, https://github.com/s9y/Serendipity/blob ... x.php#L571. The part in the config.inc.php is only echoing out the javascript, like a plugin would do, not generating the event itself.

[Timbalu]

That worked, and serendipity_editor.js was definitely not empty. If it doesn't work anymore, something else broke that

Well, I cannot say it has been working before, since I never tested the template switch this deep, before I reworked the core ckeditor access.
But I can say, while using a custom/another template it is currently not working and both js files stay empty, since not getting accessed at all.

And this is the main problem with it.

Explanation:
https://github.com/s9y/Serendipity/blob ... .php#L1051
checks for a currrently available 'serendipity_plugin_api_pre_event_hook' for execution, which is not there, or does not hold the same hook content, in case of a custom template, while this has no fallback.
But we need the serendipity_editor.js and we need the newly added hook tweaks for ckeditor.

Solution:
Now, the simplest solution for this problem is, to move 2k11/configs serendipity_plugin_api_pre_event_hook() function into the plugin_api.inc file, right underneath the function serendipity_plugin_api_backend_header() [L55], rename it to serendipity_plugin_api_corepre_event_hook() (or nicer) and execute it right before line 1051.
Thats all!

This works very well with all currently used hook-in content in that pre hook function.
I am not sure about also moving the 'js' hook prior 'jQuery('input[type="url"]')...' http/https thing function for 2k11 though.
Q: Is this new backend only, then it may move too, or is that necessary for 2k11 frontend only?
Only in the latter case, it will need to use solely a 2k11/config serendipity_plugin_api_pre_event_hook() function or simply get implemented by file or inline.

If that last question has been cleared, I will commit this solution and we can close #72.
I would love to see #102 fixed and closed too for the beta...

[garvinhicking]

Hi!

Not sure I understand most of it, but I'd be okay with the code changes that Timbalu proposes, I think when we can handle things inside the core that would be preferrable to the template doing it, especially if other backend templates would need said code as well.

Regards,
Garvin

[onli]

I'm not saying we shouldn't move things into the core. But I want to make sure that the current system is understood before.

And I think it is not a good idea to move only the serendipity_editor.js.tpl generation into the core. Do we really want to have a specific codepath listening only for that file? Can't we use something more general, and something which is not a plugin hook, if we go through the core anyway?

If the plugin api is our best bet, we could use that, but it should serve all possible files, not only serendipity_editor.js.tpl.

How it currently works:
The link to serendipity_editor.js is generated in a .tpl like this: {serendipity_getFile file='admin/serendipity_editor.js'}. serendipity_getFile in 1.7 only looked for the file itself, but I modded that function so it will look for a .tpl-file as well: https://github.com/s9y/Serendipity/blob ... c.php#L285. If it finds that, it instead generates a link to the event-plugin-hook.

Oh, and that is the current bug. It will find the template file in 2k11 and generate the wrong link, the one to the plugin-event-hook which is not loaded, if not using default as template itself.

But the point is: If that works properly, other templates don't necessarily need to add that hook. They will just fall back to the normal js-file in default, as serendipity_getTemplateFile will find that file at the end. Or, if we generalize the approach via the plugin api or something, the core will handle parsing that template file.

I am not sure about also moving the 'js' hook prior 'jQuery('input[type="url"]')...' http/https thing function for 2k11.
Q: Is this new backend only, then it may move too, or is that necessary for 2k11 frontend only?

Have a look at the js-code. That is only for url-input-fields, so they will accept links without http://. This is frontend only. It should stay in the config file.

[Timbalu]

Ok I understood you say the not processed fallback serendipity_editor.js is a bug in your code.

I just commited my solution on this, which left the input jquery thing in 2k11 config.
It now holds everything needed to hook-in for the core backend in one place, which is good for me. It even does not need the serendipity_editor.js fallback to work.

The now introduced solution works well, but if you think it is better to place the case 'external_plugin' serendipity_editor.js.tpl compilation singulary somewhere else in the core and this a better and easier to maintain solution, I can live with it.

[onli]

I thought about this a while. Turns out that I can't imagine a good system where this is generalized, so templates can generally use this approach for other files, from the core. We of course could add another Link-Pattern and detect this in index.php and generate the file then, but this is before smarty, so either initialize that or redirect it to genpage - seemed ugly.

But I fixed the bug in getFile and therefore returned the serendipity_editor.js-event to the config: https://github.com/s9y/Serendipity/comm ... b06932f67d

The fallback worked now. I think that it would better to have a generalized solution in the core, but that the specific solution only for this one file should stay in the template. Hope you agree.

[Timbalu]

Hope you agree.

NO!
Since you didn't respect all other fixes, which came along with my version, which are, simplicity, less js inline code, reduced to the least needed amount, easy working with hooked-in buttons like linktrimmer and amazonchooseer and access with all textareas like staticpages, html nugget and entryform, easy to maintain in one custom js file.
If you didn't check this to be working any better with your revert, I am totally angry with this kind of autocratic discretion!

[onli]

You already said you would be ok with this.

Btw, you are talking about a different commit.

*edit*: I'm getting sick of having the same old story every time with you. If my commit has issues, point them out calmly. You reverted and ignored part of a system I worked on for years, produced a wild mix (didn't you see serendipity[textarea]={$view}? Or did you just not care how that was supposed to work?), used design patterns which are to be avoided (a global JS variables called isinstance? wtf?) and I still tried to give your change a fair chance (and kept parts of it).

And btw, I disagree with everything you just mentioned you think is good about "your" approach.

Last thing I'll say about this. Rest through Garvin or strictly issue based.

[garvinhicking]

Hi Malte

Ian mentioned to me that the staticpage plugin no longers works with plaintext editor or CKEditor, when inserting MEDIA into its textareas after your patch. Could you have a look?

Regards,
Garvin

[onli]

Sure. If I should forget (don't know how the weekend will look like), please remind me with an issue.

Just as a note, this has nothing to do with the serendipity_editor.js replacement.

[garvinhicking]

Sure. If I should forget (don't know how the weekend will look like), please remind me with an issue.

Sure! Thanks!

Just as a note, this has nothing to do with the serendipity_editor.js replacement.

Ian said that before the last commit it worked... Haven't tested it myself yet, but I could see where his code treated those textareas differently, so this needs investigating.

Regards,
Garvin

[onli]

Ian said that before the last commit it worked... Haven't tested it myself yet, but I could see where his code treated those textareas differently, so this needs investigating.

Sure - I made two different commits around the same time, one moving the serendipity_editor.js.tpl-generation back in the template, one changing the wysiwyg-editor-init. Just so this doesn't get mixed up later.

But will sure have a look.