Tagged: , ,

Viewing 8 posts - 1 through 8 (of 8 total)
  • Author
    Posts
  • October 8, 2021 at 2:17 pm #1324115
    Jan Thiel

    Hi there,

    this is kind of similar to some already reported issues like this one: https://kriesi.at/support/topic/frontend-fehlerhaft-sektionen-farben-und-bilder/

    In our particular case we were able to identify the root cause to be caching related. Yet there are some thoughts I wanted to share with you and open up a discussion about the design decisions behind post specific CSS.

    We treat all files under /wp-content/uploads/ as “static”. Thus we ignore Query String parameters as there is no reasonable usage of them for uploaded static content. As that’s what the /wp-content/uploads/ directory is for as far as I understand the folder architecture of WordPress. On the other hand WordPress offers the /wp-content/cache/ directory for code generated content and “temporary” files. This would be the place where I would expect the avia / enfold generated files to reside in. This would follow the logic many other plugins (like WP-Rocket, W3TC, etc.) use. All other versioned code usually resides within /wp-includes/, /wp-content/plugins/ or /wp-content/themes/ and not within /wp-content/uploads/.

    Up until now we were aware of the merged CSS and JS of Enfold which was also stored in the /wp-content/upload/ directory. Yet, the files were distinguished by adding the content hash / “version” into the file name. So there was no issue with caching rules.

    I would now ask, why you implemented a second way of file versioning / cache busting. This does not make sense as is came as a surprise for us and obviously others as well.

    Post specific CSS: post-25.css?ver=ver-1633624298
    Merged CSS: avia-merged-styles-360359fae6e0b0cbb1c01af3085261a9—615f19c69ff3b.css

    There are two possible solutions I see to “align” the code either with arguable WordPress “standards” or with Enfold merged static files:
    1. WordPress “standards”: Move the generated css / js to /wp-upload/cache/avia/<domain>/<filename>.<filetype>
    -> Handling the merged CSS / JS as well as post specific CSS
    2. “Enfold Merged CSS”: Use the same cache buster for the post specific CSS as you use for the merged CSS / JS by adding an unique identifier to the filename

    This would at least allow one to tailor caching to reasonable expectations which were true before and should be true following WordPress best practices.

    Please understand this as a starting point for an open discussion as there is usually more to each decision made as is obvious in the first place.

    To solve this for now, we will disable the post specific css using the filter mentioned in the related thread. Yet as this feature does make sense and delivers some benefits, we would love to be able to re-enable it soon.

    Thanks and best regards,

    Jan

    Thanks

    October 8, 2021 at 3:28 pm #1324130
    Yigit

    Hey Jan,

    Thanks for your feedback!

    Günter has mentioned that the main reason is that the cache directory is not in clean WP install so security problems might occur when we need to create it, Elementor also has the same logic and that there should be a filter to change the location folder for the post CSS file.

    That being said, I have informed him regarding your post and he would join the conversation soon :)

    Best regards,
    Yigit

    October 11, 2021 at 1:19 pm #1324416
    Günter

    Hi,

    Thanks for your input.

    Somehow I missed our theme option “Remove query string from static resources”.

    This option does not make sense for our post css files – so I added to core to ignore it for these files.

    Lately we started to add folders to the upload directory that start with avia_ (avia_custom_shapes, avia_demo_files) and to stay consistent we used avia_posts_css.

    If you want to change to another folder structure (in the upload directory) you can use filter “avf_post_css_management_files_directory” (file enfold\config-templatebuilder\avia-template-builder\php\class-post-css-management.php).

    There is one big difference between the merged js and css files and the post css file:

    The merged files (normally) only change on a theme update which is not so often. I think this is one reason why Kriesi decided to add a hash extension.

    Post CSS files change when you update a page/post/.. and this might happen often – would mean keeping track of all versions (and deleting them again) when using hash extensions. Therefore I decided to use the same logic as Elementor with adding a version to invalidate browser cache.

    Best regards,
    Günter

    October 21, 2021 at 1:40 pm #1325871
    Jan Thiel

    Hi @guenter,

    thanks for the explanation and the hint to the filter :-) I do get your reasoning behind the file naming and the versioning using query params. That makes sense. But I still haven’t given up the opinion that these files would be better placed in /wp-content/cache. Especially as I do not think that Elementor is a good reference for many things they do. But that’s another topic and one reason why we choose Enfold over another Theme + Elementor ;-).

    Yet I especially considered @yigit s input regarding the /wp-content/cache folder not being there in “default” installation. Which is absolutely true. I took this for granted and traced back where my issue came from. The folder is being created when the Constant WP_CACHE is set to true.

    So what about instead or in addition to adding a filter to control the output folder letting the presence of the WP_CACHE variable determine whether these files are created in /wp-content/cache or /wp-content/uploads ?
    If you dislike introducing this as kind of a breaking change automatically, what about a filter controlling this? Like avia_store_generated_files_in_cache_folder which defaults to false.

    Personally I believe this might be a way allowing for a more fine granular control. What do you think?

    Best regards,

    Jan

    October 21, 2021 at 3:49 pm #1325904
    Günter

    Hi Jan,

    Thank you for your feedback.

    I added

    
define( 'WP_CACHE', true );

    to wp-config.php on my localhost server (WP 5.8.1, PHP 8.0).

    On my install this does nothing – cache directory is not created.

    After installing and activating “WP Super Cache” the directory is created. When I deactivate this plugin the cache directory is deleted even if I add a folder with content inside the cache directory.

    This behaviour does not allow to rely on the cache folder.

    What I can think of are 2 filter where you can set a full path in your file system and the url to this folder. But you have to make sure that folder permissions are correct and the folder is not deleted by other plugins.

    Best regards,
    Günter

    October 21, 2021 at 8:50 pm #1325943
    Jan Thiel

    I just double check and you are right. Don’t know what went wrong in my DEV env … most certain stale folders from previous test runs with plugins.

    Fun fact: There is actually one reference to this particular cache directory in the WordPress sources: https://github.com/WordPress/wordpress-develop/blob/1cf97a3c1479f90a86450f19bd361c71f4b999e3/src/wp-includes/rss.php#L721
    Yet the code ist 14 years old ;-)

    If you can make the two additional filters work to allow paths outside the uploads directory, that would be amazing. And yes it’s on us to care for proper folder access rights and stuff :-)

    Thank you very much for your patience and double checking this!

    Best regards,
    Jan

    October 25, 2021 at 10:04 am #1326273
    Jan Thiel

    Good Morning @guenter,

    Just a follow up from my side. On Friday we actually changed our caching configuration to align with avia files being in wp-content/uploads. There was one particular thing which I would open up for discussion (again). We figured out that there are now several folders for enfold / avia content directly below /uploads/.

    Am I correct that there are at least these folders?
    ./avia_fonts
    ./avia_posts_css
    ./dynamic_avia

    Considering that we require a reliable path to configure the caching for (consider /wp-content/uploads/enfold/*), this makes life quite hard. Especially if new folders are added from time to time. Like it recently happened with the avia_posts_css.
    Would it be possible to at least group these folders inside one parent folder? This would allow one to easily catch all current and future(!) folders you create. With the current setup any cache exclusions would have “exploded” when you added the avia_posts_css folder.

    We also investigated the filter you mentioned and this would actually not solve the issue with new folders. So whenever you add a new folder we would need to add a new filter to align this with our folder structure. This does not really scale or work reliably.
    Talking about filters, this would actually require one global filter which is then also reliably implemented for new folders – yet posing the risk when you miss that, that caching again explodes for custom configs. This filter could be responsible for creating the common parent folder.

    All the filter stuff seems kind of unstable due to the reasons mentioned. Yet I totaly understand that you hesitant changing the folder structures…

    And sorry for all the ping pong. We needed some time digging into this and try to find a stable and scalable solution which doesn’t make you work and code more complex than it already is :-)

    What do you think of this approach?

    Best,
    Jan

    October 27, 2021 at 1:04 pm #1326665
    Günter

    Hi Jan,

    Agree. Makes sense to group the folders below /wp-content/uploads/enfold/ to have a clear directory structure.

    I add it to the dev repo and will check how we can move the existing folders without breaking existing sites.

    The avia_posts_css is no problem as it is cleared after each theme update or when saving theme options.

    I will let you know here about the progress.

    Thank you very much for your ideas

    Best regards,
    Günter

  • Author
    Posts
Viewing 8 posts - 1 through 8 (of 8 total)
  • You must be logged in to reply to this topic.