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
Comment | File | Size | Author |
---|---|---|---|
#3 | defaultpluginmanager-2480307-3.patch | 3.33 KB | camoa |
Comments
Comment #1
camoa CreditAttribution: camoa at Camo Advanced Tech commentedHere is an Idea, the providerExist method can be overridden by plugin managers that want themes to provide definitions.
Comment #2
tim.plunkettThis is even better than what we discussed in IRC, but I think it should include a usage of this in BreakpointManager.
Comment #3
camoa CreditAttribution: camoa at Camo Advanced Tech commentedGood point!
Here it is. An example replacing the findDefinitions in BreakpointPluginManager and reducing code redundancy.
Comment #4
dsnopekOoo, 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.
Comment #5
camoa CreditAttribution: camoa at Camo Advanced Tech commentedComment #6
lostkangaroo CreditAttribution: lostkangaroo commentedI don't think this needs a beta evaluation due to its nature as a bug, confirmed by Alex.
Comment #7
alexpottBreakpoints 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 thefindDefinitions()
method.Committed 3d7cd4e and pushed to 8.0.x. Thanks!