Add a configuration switch for enabled preview mimetypes #11211
Conversation
|
Closes #11170 |
|
the right approach imho |
|
👍 |
|
For what it is worth, I agree. Good approach. |
| * - OC\Preview\PDF | ||
| * - OC\Preview\Tiff | ||
| */ | ||
| $enabledProviders = \OC::$server->getConfig()->getSystemValue('enabledPreviewProviders', array( |
There was a problem hiding this comment.
One DB query for each registerProvider call? Why not put it into a static variable? Don't mind, that static stuff is evil, it is even a static method - and once this is refactored to a class instance it could easily be made non-static.
There was a problem hiding this comment.
yes. static configuration should be totally fine in this case because the code implementation is also static ;-)
LukasReschke
force-pushed
the
previewProviderSwitch
branch
from
8652c32
to
bc17aa9
Compare
LukasReschke
force-pushed
the
previewProviderSwitch
branch
from
bc17aa9
to
ca3447f
Compare
|
@MorrisJobke @karlitschek Static now and good to review. Thanks. Requires a backport down to stable6 and also needs to be documented. @carlaschroder All preview providers except "Image", "MP3", "TXT" and "MarkDown" are now disabled by default due to performance and privacy concerns. They can be enabled by a switch in |
|
💣 Test FAILed. 💣 |
|
Let me check why the hell we have VCF tests in the preview ... |
|
@georgehrke I have no idea what the VCF and ICS test in there are for. Please take a look. Thanks. (Please review regardless of this test fail…) |
It seems like those files where supposed to test the transparent background of the TXT backend, but afaik looking at it the vcf and ics didn't test the transparent background of the txt backend (as supposed), but rather the transparent backend of the unknown backend (which is gone now, that's why the test fail). I'd say just remove the ics/vcf file from the data-provider (and delete the files if they are not used in any other tests)
Code looks good, testing right now |
Done - thanks.
Awesome :-) |
|
Works 👍 Images, mp3 and txt work out of the box. Other preview backends don't, but work just fine after the enabling them in the config.php |
|
👍 Please backport |
|
🚀 Test PASSed. 🚀 |
|
🚀 Test PASSed. 🚀 |
Add a configuration switch for enabled preview mimetypes
|
@LukasReschke Can I ask you to backport this? Thanks |
Backport of #11211 to stable7
|
Stable7 PR: #11245 |
Backport of #11211 to stable6
|
Stable6: #11246 |
|
The inspection completed: No issues found |
|
What are the other preview providers, and what are the exact options to use in config.php? Are "Image", "MP3", "TXT" and "MarkDown" the actual config switches? @LukasReschke |
|
Also, does 'preview_office_cl_parameters' go away? |
See config.sample.php: https://github.com/owncloud/core/blob/master/config/config.sample.php#L297
No |
|
Excellent, thanks @georgehrke |
Do not merge - requires discussion and some fine-tuning.
@MTRichards @karlitschek @craigpg See mail. THX.