Minify adjustments #3473
Minify adjustments #3473
Conversation
…, so this can be removed as it is depurated in PHP 7. Fix FTP connection needing a login token on upgrader. Require upgrader to make sure minified files can be wrote to. Have SMF retry css/js processing if it can't minify, to prevent broken forums Signed-off-by: jdarwood007 <unmonitored+github@sleepycode.com>
|
So I upgraded another one of my many test forums to the latest 2.1 and it broke afterwards due to being unable to write minify files. The adjustments to minify process here has it bail out of minify and load regular files if it can't write. This isn't checking to see if it can minify for a single theme only or not. It simply bails out. Somebody can work that in but it requires some rewrites of the minify process. This makes the process of minify much safer as the minify process failing causes the forum to be unusable. |
|
Someone should really merge this. |
| @@ -3438,7 +3448,7 @@ function custMinify($data, $type, $do_deferred = false) | |||
| { | |||
| loadLanguage('Errors'); | |||
| log_error(sprintf($txt['file_not_created'], $toCreate), 'general'); | |||
| continue; | |||
| return false; | |||
There was a problem hiding this comment.
How will this behave with multiple themes?
Lets say there are 3 active themes and the second one fails, with a return the third one will never be processed.
|
With multiple themes based on how I understand the minify code works, it would fail for the current theme? I believe it rolls all css into 1 file even for multiple themes? We can add a minified_defer.js if needed. The fix at least makes the forum usable if minify process fails. What defer items are broken by this? If this PR isn't merged, we need to resolve this as it is a forum breaking issue that holds up the release as when the minify process fails the forum is not usable (no css or js). |
|
On Fri, 10 Jun 2016 19:27:48 -0500, Jeremy D notifications@github.com
None, I was solely talking about adding that file to that list.
Well yes, thing is, support for multi themes is a must, that needs to be |
|
I'm not sure what to do for multiple theme support. The main part of this code ties into the already existing part of the minify process that reports an error when it can't minify and returns in a way that we know it failed and can exit safely. |
|
I've been thinking about moving the echo part into the function so there's no need to return any value and we can still have control over the way the minimized files are created and we still have control if the process fails and make sure there's a safe fallback Jeremy D notifications@github.com wrote:
|
|
Well if you got a plan then we can hold off on this PR or merge this and then you commit changes over it. |
|
Will do when I get home Jeremy D notifications@github.com wrote:
|
|
So yeah that return needs to be changed back to a continue, multiple themes support is currently broken. |
Add a filesize check after the minify process fixes SimpleMachines#3480 SimpleMachines#3473 Signed-off-by: Jessica González <suki@missallsunday.com>
|
I was going to say that the minify process didn't really handle multiple themes properly from what I could tell. But I see your changes and that should work. |
Using methods with the same name as their classes is needed for PHP 4 so this can be removed as it is depurated in PHP 7.
Fix FTP connection needing a login token on upgrader.
Require upgrader to make sure minified files can be wrote to.
Have SMF retry css/js processing if it can't minify, to prevent broken forums
Signed-off-by: jdarwood007 unmonitored+github@sleepycode.com