Copyright not showing on mobile theme when user signs out.

peregrine

@vrijvlinder said: What I would like to know is where does it check for the Value of MobileFriendly. If it does not say MobileFriendly at all it gets removed in theory correct? Based on the plugin manager file.

if the method (function) is called - public function RemoveMobileUnfriendlyPlugins()

then here:

library/core/class.pluginmanager.php:461: if (!GetValue('MobileFriendly', $PluginInfo))

https://github.com/vanilla/vanilla/blob/2.1/library/core/class.pluginmanager.php#L461

@vrijvlinder said: If it says MobileFriendly False nothing happens. In some cases even mobilefriendly true does not work either.

it depends on how GetValue evaluates - but we went through "False" vs FALSE vs TRUE vs "TRUE" vs "FALSE" boolean vs string before

@vrijvlinder said: I don't think it should be left up to the themehooks of a theme to remove the plugins. I think the plugin should dictate if it is or not friendly and specify it. And the manager should remove it based on the value true or false of mobilefriendlyness.

Otherwise the function is useless if a theme does not specify to remove them.

yes if the non-default mobile theme does not call the function RemoveMobileUnfriendlyPlugins()
it won't be removed - tautology or no tautology.

vrijvlinder

Yes

library/core/class.pluginmanager.php:461: if (!GetValue('MobileFriendly', $PluginInfo))

The value is gotten from the plugin info correct ? therefore it should remove the plugin if mobile and for sure if it says MobileFriendly=>False regardless of the themehooks.

Bleistivt

@vrijvlinder said:
Yes

library/core/class.pluginmanager.php:461: if (!GetValue('MobileFriendly', $PluginInfo))

The value is gotten from the plugin info correct ? therefore it should remove the plugin if mobile and for sure if it says MobileFriendly=>False regardless of the themehooks.

Correct, but the function is never called by the core
https://github.com/vanilla/vanilla/search?utf8=✓&q=RemoveMobileUnfriendlyPlugins

It is only called by the default mobile theme.

RemoveMobileUnfriendly doesn't differentiate between ACCESS_PLUGINNAME and ACCESS_CLASSNAME, it just assumes that classname = pluginname + 'plugin'

GetPluginInfo() on the other hand does that, which is why any kind of classname will work, it just won't disable on mobile.

vrijvlinder

GetPluginInfo() on the other hand does that, which is why any kind of classname will work, it just won't disable on mobile.

Logically it should don't you think ?.

Assuming that author knows it is not friendly and therefore adds my line in there to make sure, that function (Mobilefriendly=>False; or omits this) is useless unless there is a themehooks file or something else to make sure.

Agreed ?

Bleistivt

Yes, I think that behaviour should be changed, too.

This would also be better for dynamic themes that work on desktop and mobile

R_J

@Bleistivt said:
3. The plugins classname ends with "Plugin"

The second one is debatable, but I'd say the last one is a bug.

I have seen many plugins that don't have classnames ending with "Plugin", there are even 2 in the official addons repo (Vanoogle and CssThemes)

I'd prefer if plugins can only be enabled if they adhere to that naming convention. I dislike the lack of conventions concerning file names and directory structure. I agree that this is no problem most of the time, but as it could be seen in this example, developers rely on special conventions and if they are not met, you'll end up with a bug that is hard to track down.

Bleistivt

@R_J‌ Yes, that would avoid confusion. It should be stated more clearly in the docs.

Still, there is the concept of ACCESS_PLUGINNAME and ACCESS_CLASSNAME throughout the pluginmanager, so the support for different classnames is intentional.

Linc

Shoutout to @Bleistivt‌ for submitting PR 2121 (now merged) to correct this.