Skip to content

Conversation

@nickvergessen
Copy link
Contributor

Regression from: #18109

@DeepDiver1975 @Xenopathic @LukasReschke

This broke all OCS routes of apps => no smashbox possible anymore.

@scrutinizer-notifier
Copy link

A new inspection was created.

@LukasReschke
Copy link
Member

I'm wondering whether we should not just revert #17961 and move the required logic to lib/private/app.phpin L434. I really can't judge if this will fix all edge-cases or if there still will be things that break.

@LukasReschke
Copy link
Member

Basically do a check for the configured theme URL in app.php (just check if the configured value is empty) and if not do the linking there just as before. Then the theme class does not require the URLGenerator.

@RobinMcCorkell
Copy link
Member

I would suggest not even loading any OCS routes if there are unavailable apps. With this approach funky things might happen when OCS routes are reloaded once apps become available, especially with $this->root->addCollection() being called multiple times with duplicated content. At best, this can become a performance issue, at worst, the duplicate routes confuse something else to the point of breakage.

@ghost
Copy link

ghost commented Aug 11, 2015

🚀 Test PASSed.🚀
chuck

@DeepDiver1975 DeepDiver1975 deleted the fix-loading-of-app-ocs-routes branch August 11, 2015 12:22
@DeepDiver1975
Copy link
Member

let's revert this mess and retry from scratch

@DeepDiver1975
Copy link
Member

@nickvergessen will open a revert pr - THX

@lock lock bot locked as resolved and limited conversation to collaborators Aug 11, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants