Please upgrade here. These earlier versions are no longer being updated and have security issues.
HackerOne users: Testing against this community violates our program's Terms of Service and will result in your bounty being denied.

Issue: Gdn_Plugin::getResource() useless in 2.2b1 due to changes in Gdn_Controller::renderMaster()

mtschirsmtschirs ✭✭✭
edited July 2015 in Vanilla 2.0 - 2.8

The Gdn_Plugin::getResource() method was introduced to simplify adding of CSS as well as JS files via Gdn_Controller::addCssFile() and Gdn_Controller::addJsFile().

Without Gdn_Plugin::getResource(), a plugin would have to include a CSS file as follows:

$Sender->addCssFile('design/style.css', 'plugin/PluginFolder');

With Gdn_Plugin::getResource(), a plugin can leave out the second argument (the application folder):

$Sender->addCssFile($this->getResource('design/style.css'));

The Gdn_Plugin::getResource() method would automatically prepend the plugin folder and return 'plugin/PluginFolder/design/style.css'.

However, this doesn't work any more in Vanilla 2.2b1 since the Gdn_Controller::renderMaster() method was changed to look for the given CSS or JS files within its own application folder if the second argument of Gdn_Controller::addCssFile() (the application folder) was left out.

This creates a compatibility issue and renders the Gdn_Plugin::getResource() convenience method practically useless.

The relevant changes might have been introduced with this commit: https://github.com/vanilla/vanilla/commit/c6d013283ec8ef19608abd819bdbacccefeb0bbb

Any ideas on how to fix this?

Comments

  • BleistivtBleistivt Moderator
    edited July 2015

    GetResource uses absolute paths with the default parameters, which have been deprecated in the AssetModel.

    I'd just use addCssFile with the second argument. This is how the majority of official plugins do it.
    You can omit the design folder:

    $Sender->addCssFile('style.css', 'plugins/PluginFolder');
    

    Alternatively, this should work:

    $Sender->addCssFile($this->getResource('design/style.css', false, false));
    
  • mtschirsmtschirs ✭✭✭
    edited July 2015

    Alternatively, this should work:

    $Sender->addCssFile($this->getResource('design/style.css', false, false));
    

    Unfortunately not. The default values for the optional second and third argument are already false.

    The easiest fix would be to remove Gdn_Plugin::getResource(). Some plugins that rely on it - such as the Discussion Polls plugin - will have to be updated.

  • BleistivtBleistivt Moderator

    @mtschirs said: Unfortunately not. The default values for the optional second and third argument are already false.

    I just checked and at least in 2.2 this works (the third params default is true).

    But it will throw a notice AssetModel::CssPath() with direct paths is deprecated.

  • mtschirsmtschirs ✭✭✭
    edited July 2015

    Ah, sorry, you are right, the last default value is true. However, in the current master branch of 2.2 resources where not included even with

    $Sender->addCssFile($this->getResource('design/style.css', false, false));
    

    I will investigate more and report back...

  • mtschirsmtschirs ✭✭✭

    Confirmed. This issue also exists within the Flagging plugin (see https://github.com/vanilla/vanilla/issues/3015):

    $Sender->addCssFile($this->getResource('design/flagging.css', false, false));
    is equal to
    $Sender->addCssFile('plugins/Flagging/design/flagging.css');
    which adds the following entry to Controller->_CssFiles:

    Array(
      [FileName] => plugins/Flagging/design/flagging.css
      [AppFolder] => 
      [Options] => 
    )
    

    Controller->renderMaster() then does the following in line 1711:

    $AppFolder = $CssInfo['AppFolder'];
    $LookupFolder = !empty($AppFolder) ? $AppFolder : $this->ApplicationFolder;
    

    As a result, $LookupFolder is set to e.g. 'vanilla' and the CSS lookup in AssetModel::CssPath($CssFile, $LookupFolder, $ThemeType) fails.

  • I use $Sender->addCssFile('plugins/Flagging/design/flagging.css'); habitually because I noticed a problem with getResource , I blamed it on me .... :(

Sign In or Register to comment.