Skip to content
This repository has been archived by the owner on Nov 12, 2017. It is now read-only.

Don't include JS-lib by minified name. #98

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

enoch-root
Copy link

Roundcube finds those minified versions of the file by itself, while including it literally by the minified name breaks inclusion with other minification strategies (or no minification at all).

Roundcube finds those minified versions of the file by itself, while including it literally by the minified name breaks inclusion with other minification strategies (or no minification at all).
@johndoh
Copy link
Owner

johndoh commented Dec 16, 2014

This creates a new problem and the current code is correct – only the minified version is distributed with the plugin.

I know there are extra checks in include_script() which check for a minified version of the file. I believe this is designed for when Roundcube is put into production and the normal files are minified. The inclusion of sieverules.js replies on this for example.

What I think you are suggesting it so change the line so that it will never work on its own, instead relying on extra work done in the include_script() function. I see 2 problems with this:

  1. If you run Roundcube in devel_mode checks for minified JS files are always skipped - and so the JS file would never be included and the plugin would not work correctly.

  2. No normal version of this JS lib is distributed with the plugin and so trying to include it is just wrong - for example if something was to change in include_script() about how the minified files check works then the plugin may stop working.

@enoch-root
Copy link
Author

I agree my suggestion would break devel_mode, so I withdraw it in this form.

Instead I'd like to ask you to distribute non-minified JavaScript-code with your plugin, too. That would enable providers the choice to optionally serve readable code.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants