Problem/Motivation

There are some plugins that are related to Front End, like Breakpoints and Layout_plugin. This plugins should allow themes to add definitions when needed.

The findDefinitions method on the DefaultPluuginManager check for the existance of the provider as a method, this leads the method to unset any definition find inside themes.

To workaround this the Breakpoint plugin and Layout_plugin are taking the path of duplicating the findDefinitions, pretty much the same code but adding an extra check for themes; this leads to duplicated code and in general not really good practice.

Proposed resolution

There are some ways to do this:
1. in findDefinitions: check if the pluginmanager has a themehandler and then add a check for themeexists().
2. Add a method ProviderExists(provider) that can be overridden by the pluginmanager and returns true if the theme or module exist, just the module by default. (Based on Tim Plunkett's idea)
3. just simply always allow definitions if provider is an existing theme.

Remaining tasks

a. Decide on a method.
b. Do the simple fix for that, I pesonally like the second one and will provide a patch soon.

User interface changes

None

API changes

None

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

camoa’s picture

Here is an Idea, the providerExist method can be overridden by plugin managers that want themes to provide definitions.

tim.plunkett’s picture

This is even better than what we discussed in IRC, but I think it should include a usage of this in BreakpointManager.

camoa’s picture

Good point!

Here it is. An example replacing the findDefinitions in BreakpointPluginManager and reducing code redundancy.

dsnopek’s picture

Status: Active » Needs review
Issue tags: +Needs beta evaluation

Ooo, yes, +1 to this patch! I agree with Tim that it's even better than what we discussed in IRC. :-)

This needs a beta evaluation added to the issue summary in order to get the committers to consider it:

https://www.drupal.org/core/beta-changes

Setting to "Needs review" so the testbot will give it a go.

camoa’s picture

lostkangaroo’s picture

Status: Needs review » Reviewed & tested by the community

I don't think this needs a beta evaluation due to its nature as a bug, confirmed by Alex.

alexpott’s picture

Status: Reviewed & tested by the community » Fixed
Issue tags: -Needs beta evaluation

Breakpoints provided by themes is tested in the breakpoint module. I classify this as a bug since implementing providerExists() is way better and less risky than overriding the findDefinitions() method.

Committed 3d7cd4e and pushed to 8.0.x. Thanks!

  • alexpott committed 3d7cd4e on 8.0.x
    Issue #2480307 by camoa: DefaultPluginManager doesn't allow Plugin...

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.