Page 2 of 4

Re: 2.1Beta: Infobox when saving a new article is not shown

Posted: Tue Sep 06, 2016 10:06 am
by yellowled
It seems there's a larger issue at hand here. The iframed preview in Next does not load a lot of the frontend assets required for the preview, which seems to boil down to serendipity_getFile not working properly in the preview iframe/in preview_iframe.tpl. The files that are not loaded are

* oldie.css
* scripts/modernizr/modernizr.js
* scripts/master.js

All these are supposed to be loaded via serendipity_getFile in preview_iframe.tpl, and that does not work. It doesn't even throw a 404, the href/src attributes just remain empty. I only noticed because it throws an error that the Modernizr object is not defined.

However, stuff like <script src="{serendipity_getFile file='admin/js/plugins.js'}"></script> still works. This is not even specific for Next, the same thing happens with js/clean-blog.js in Clean Blog, which is referenced the same way.

Haven't there been recent changes to serendipity_getFile? Because I'm pretty sure this used to work properly before. And it's entirely possible that this influences the calculation of the preview's height, so I think this should be resolved first.

YL

Re: 2.1Beta: Infobox when saving a new article is not shown

Posted: Tue Sep 06, 2016 10:09 am
by yellowled
yellowled wrote:Haven't there been recent changes to serendipity_getFile? Because I'm pretty sure this used to work properly before.
Can confirm this still worked properly in 2.0.3. My blog runs 2.0.3, the preview_iframe.tpl there uses the same references to frontend assets, does not throw errors.

YL

Re: 2.1Beta: Infobox when saving a new article is not shown

Posted: Tue Sep 06, 2016 10:42 am
by onli
serendipity_getFile uses the plugin/template fallback scheme, and that got changed as it was broken before (when the preview did not work at all).

The files not loaded are all in templates/next, or in 2k11, and next is the activated theme?

Re: 2.1Beta: Infobox when saving a new article is not shown

Posted: Wed Sep 07, 2016 12:24 pm
by yellowled
onli wrote:The files not loaded are all in templates/next, or in 2k11, and next is the activated theme?
Next is the active frontend theme with 2k11 as the active backend theme.

The iframed entry preview is supposed to load, but does not load

* /templates/next/oldie.css → {serendipity_getFile file="oldie.css"})
* /templates/next/scripts/modernizr/modernizr.js → {serendipity_getFile file="scripts/modernizr/modernizr.js"}
* /templates/next/scripts/master.js → {serendipity_getFile file="scripts/master.js"}

For all these references in the iframed preview's markup, the href or src attributes are just empty (src="" or href=""), which is why they don't even throw a 404, so I would asssume that serendipity_getFile just returns “nothing”. The other serendipity_getFile instances in preview_iframe.tpl which load assets from /templates/2k11/admin/ work just fine in Next's preview_iframe.tpl.

Reproduced in Serendipity 2.1-beta1 on PHP 5.6.21.

YL

Re: 2.1Beta: Infobox when saving a new article is not shown

Posted: Wed Sep 07, 2016 1:49 pm
by onli
Okay.

It is easy to see why getFile does not find those – it is called in the backend, and there is nothing telling it to look in the frontend. It is *not* easy to see why that is. It means that before, there was somewhere magic to make getTemplateFile look in the frontend template.

My guess is $serendipity['smarty_preview']. That variable is not set anymore, but was in the old getTemplateFile implementation.

We have two options now:

1. Re-introduce some magic that makes getTemplateFile look in the frontend when referencing files in preview_iframe.tpl. That could possibly mean search where smarty_preview was set and using that. Has the potential issue that the code in the old getTemplateFile is not at all clear, that variable could do something else.
2. Give power to the template. getFile could have a parameter to trigger the frontend fallback.

2 is what I tested now and which works fine. Looks like this:

Code: Select all

{serendipity_getFile file="scripts/master.js" frontend=true}
It is a one-line patch fo serendipity_smarty_getFile.

I think it is the right solution. It makes it immediately clear where getFile is supposed to look, without introducing hidden complexity.

Tell me what you think, I'll wait for at least your comment before I commit this.

Re: 2.1Beta: Infobox when saving a new article is not shown

Posted: Wed Sep 07, 2016 2:46 pm
by yellowled
onli wrote:Looks like this:

Code: Select all

{serendipity_getFile file="scripts/master.js" frontend=true}
It is a one-line patch fo serendipity_smarty_getFile.
I assume this means we only have to set frontend=true if the asset is from the frontend theme's directory, otherwise it will get it from the backend theme's directory? Are there no occurrences of serendipity_getFile in other contexts such as plugins?

If so, I like this.

Can we assume that only preview_iframe.tpl files that use serendipity_getFile to get assets from the theme directory will need this? What will happen if a theme with a tpl tile that already uses frontend=true is used in s9y < 2.1?

YL

Re: 2.1Beta: Infobox when saving a new article is not shown

Posted: Wed Sep 07, 2016 3:21 pm
by onli
yellowled wrote:I assume this means we only have to set frontend=true if the asset is from the frontend theme's directory, otherwise it will get it from the backend theme's directory?
Yes, partly. Otherwise it will get it from where it is – from the backend when in the backend, from the frontend when in the frontend.

Maybe this would be clearer if we call if force_frontend, to make sure it is an ovverride and not always necessary?
yellowled wrote:Are there no occurrences of serendipity_getFile in other contexts such as plugins?
There are, but those are not necessary in the backend context and expecting a .tpl from the frontend.
yellowled wrote:Can we assume that only preview_iframe.tpl files that use serendipity_getFile to get assets from the theme directory will need this?
Yes, because in the other cases the integrated check if we are in the backend or frontend will work fine.
yellowled wrote:What will happen if a theme with a tpl tile that already uses frontend=true is used in s9y < 2.1?
It will continue to work fine. serendipity_smarty_getFile has an $params-array, and in that array will be set $params['frontend'], but the old code does nothing with that and also won't be disturbed by it. It just looks at $params['file'].

Re: 2.1Beta: Infobox when saving a new article is not shown

Posted: Thu Sep 08, 2016 9:35 am
by yellowled
onli wrote:Maybe this would be clearer if we call if force_frontend, to make sure it is an ovverride and not always necessary?
I think chances are that future themes will have to load backend theme and frontend theme assets in the preview. Especially once http/2 hits and no one will ever combine assets. :roll: If we were to rename it in order to make it clearer and less confusing, I guess I'd make it a required attribute tpldir=frontend|backend or something.
onli wrote:It will continue to work fine. serendipity_smarty_getFile has an $params-array, and in that array will be set $params['frontend'], but the old code does nothing with that and also won't be disturbed by it. It just looks at $params['file'].
Okay, so we can just switch all themes in the core and additional_plugins after committing this to master. Very good.

YL

Re: 2.1Beta: Infobox when saving a new article is not shown

Posted: Thu Sep 08, 2016 10:21 am
by onli

Re: 2.1Beta: Infobox when saving a new article is not shown

Posted: Thu Sep 08, 2016 6:17 pm
by yellowled
onli wrote:Pushed, see
Works for me. I'll commit fixes for the other themes in core and additional and then address the original issue in Next's preview later.

YL

Re: 2.1Beta: Infobox when saving a new article is not shown

Posted: Thu Sep 08, 2016 7:40 pm
by yellowled
yellowled wrote:I'll commit fixes for the other themes in core and additional and then address the original issue in Next's preview later.
I have committed fixes for all core and additional themes and fixed the original issue of this thread, the height of the iframe preview. I also, along the way, may have found a way to greatly simplify this stupid preview_iframe.tpl overall (just need to think about it a little if it has side effects).

As you may have guessed already, there's a “but”.

i stumbled across what I believe is yet another bug in the template fallback chain. To reproduce it, set Next as the active theme, clear the template cache to be safe, generate a preview of any entry and check out that preview in the dev tools of your browser. Specifically, look at the markup for, for instance, the entry byline. In my dev blog, for example, this is

Code: Select all

<span class="serendipity_byline block_level"><span class="single_user">Geschrieben von <a href="http://s9y.netzgestaltung.net/authors/1-Matthias-Mees">Matthias Mees</a> am </span><time datetime="2016-02-09T13:16:00+01:00">Dienstag,  9. Februar 2016</time></span>
That is not the markup used in Next for that byline, it's the markup used in 2k11. Which is why it is rendered wrong in the iframed backend preview. I assume (but haven't checked due to time constraints) that this will look even worse in other themes that are more different from 2k11 than Next is (which also explains why I didn't notice that earlier).

Judging from the markup in the backend preview's iframe, I assume that Next's preview_iframe.tpl for some odd reason still uses 2k11's {$preview}.

This is where you come in because I can not even debug this. :)

YL

Re: 2.1Beta: Infobox when saving a new article is not shown

Posted: Fri Sep 09, 2016 2:37 pm
by onli
Okay. Ja, die Preview stimmt nicht. Und die einfachste Erklärung ist, dass die Preview die falsche entries.tpl nimmt. ich schaus mir an.

Re: 2.1Beta: Infobox when saving a new article is not shown

Posted: Fri Sep 09, 2016 2:59 pm
by onli
Nicht so einfach zu debuggen, die Preview zieht sich ziemlich durch den Kern, und der Aufruf im iframe ist indirekt. Der Fehler war, dass die Preview von serendipity_printEntries generiert wird, indem es serendipity_smarty_fetch aufruft. Die Funktion nutzt dann serendipity_getTemplateFile, aber ohne den force_frontend-Parameter zu setzen. Es fand also die entries.tpl des Backend-Templates. Fix in https://github.com/s9y/Serendipity/comm ... 31b5948d7d, wobei ich die Variable noch umbennen werde.

Re: 2.1Beta: Infobox when saving a new article is not shown

Posted: Fri Sep 09, 2016 3:58 pm
by yellowled
Great, works perfectly for me. Nice “three birds with one stone” bugfixing. :mrgreen:

YL

Re: 2.1Beta: Infobox when saving a new article is not shown

Posted: Fri Sep 09, 2016 4:52 pm
by yellowled
Not the same thing, but related: Most preview_iframe.tpl files now have

Code: Select all

<script src="{serendipity_getFile file='admin/js/plugins.js'}"></script>
<script src="{serendipity_getFile file='admin/serendipity_editor.js'}"></script>
Can you remind me why we need this in the <head> of the backend preview iframe? Because it at least throws an error in themes that don't use Modernizr.

YL