Uploaded image for project: 'phpBB3'
  1. phpBB3
  2. PHPBB3-11727

INCLUDECSS and INCLUDEJS incorrectly default to /template/ folder

    Details

    • Type: Bug
    • Status: Unverified Fix
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: 3.1.0-dev
    • Fix Version/s: 3.1.0-a1
    • Component/s: Template Engine
    • Labels:
      None

      Description

      Currently INCLUDECSS and INCLUDEJS default to searching inside style/template/... for files.

      CSS files are not inside the template folder, they are in "theme" folders. So INCLUDECSS can not work at this time.

      Perhaps the strategy would be to default INCLUDEJS to template folders, and INCLUDECSS to theme folders.

      But Extensions must also be considered. Maybe for extensions (namespaced) we should not go deep into template or theme folders by default, and require the authors to use template/ or theme/ in their include tags.

      Ideally, we should be able to do stuff like this:

      CORE:
      <!-- INCLUDEJS file.js --> to find: styles/(style match)/template/file.js

      <!-- INCLUDECSS file.css --> to find: styles/(style match)/theme/file.css

      Extensions:
      <!-- INCLUDEJS @foo_bar/template/file.js --> to find: ext/foo/bar/styles/(style match)/template/file.js

      <!-- INCLUDECSS @foo_bar/theme/file.css --> to find: ext/foo/bar/styles/(style match)/theme/file.css

      and that would also allow for this:
      <!-- INCLUDEJS @foo_bar/assets/plugin.js --> to find: ext/foo/bar/styles/(style match)/assets/plugin.js

        Issue Links

          Activity

          Hide
          EXreaction EXreaction [X] (Inactive) added a comment -

          Currently, the style/template/ path is given to the template engine.

          To do this, we need to give the style/ path to the engine and add directories based on what type of file is loaded/through what method.

          Show
          EXreaction EXreaction [X] (Inactive) added a comment - Currently, the style/template/ path is given to the template engine. To do this, we need to give the style/ path to the engine and add directories based on what type of file is loaded/through what method.
          Hide
          EXreaction EXreaction [X] (Inactive) added a comment -

          Figuring out the exact correct path is a bit difficult with the current design.

          phpBB styles are easy, they should always be ../theme/
          custom styles are impossible to tell, e.g. in adm/style/ it will be ./

          To manage this properly we'd need a new styles system to take input for the template path and theme path for any given style, which seems as it may be overkill if only to save from people having to write ../theme/ for INCLUDECSS. Plus we want to move to assetic in the future, which may obsolete any implementation we create now.

          The biggest issue now is that INCLUDECSS doesn't work with ../theme/* as the loader doesn't allow it (loading outside of the template directory).

          The PR for this sets up the ability to specify "safe" directories to load files within and will make all the phpBB style subdirectories and their children safe (e.g. styles/prosilver/*) to load files from. After this PR, INCLUDECSS ../theme/common.css should work.

          Show
          EXreaction EXreaction [X] (Inactive) added a comment - Figuring out the exact correct path is a bit difficult with the current design. phpBB styles are easy, they should always be ../theme/ custom styles are impossible to tell, e.g. in adm/style/ it will be ./ To manage this properly we'd need a new styles system to take input for the template path and theme path for any given style, which seems as it may be overkill if only to save from people having to write ../theme/ for INCLUDECSS. Plus we want to move to assetic in the future, which may obsolete any implementation we create now. The biggest issue now is that INCLUDECSS doesn't work with ../theme/* as the loader doesn't allow it (loading outside of the template directory). The PR for this sets up the ability to specify "safe" directories to load files within and will make all the phpBB style subdirectories and their children safe (e.g. styles/prosilver/*) to load files from. After this PR, INCLUDECSS ../theme/common.css should work.
          Hide
          Marc Marc added a comment -

          INCLUDECSS still looks in the template folder of extensions:

           Uncaught exception 'Twig_Error_Loader' with message 'Unable to find template "portal.css" (looked into: ./ext/board3/portal/styles/prosilver/template) in "@board3_portal/event/overall_header_head_append.html" at line 2.'

          One has to use this to make it look in the theme folder:

          <!-- INCLUDECSS ../theme/portal.css -->

          Show
          Marc Marc added a comment - INCLUDECSS still looks in the template folder of extensions: Uncaught exception 'Twig_Error_Loader' with message 'Unable to find template "portal.css" (looked into: ./ext/board3/portal/styles/prosilver/template) in "@board3_portal/event/overall_header_head_append.html" at line 2.' One has to use this to make it look in the theme folder: <!-- INCLUDECSS ../theme/portal.css -->
          Hide
          EXreaction EXreaction [X] (Inactive) added a comment -

          See comment just before yours.

          You should be able to use

          {T_THEME_PATH}

          portal.css if it makes more sense

          Show
          EXreaction EXreaction [X] (Inactive) added a comment - See comment just before yours. You should be able to use {T_THEME_PATH} portal.css if it makes more sense
          Hide
          Marc Marc added a comment -

          No, <!-- INCLUDECSS

          {T_THEME_PATH}

          /portal.css --> only looks inside prosilver and not the prosilver folder of the extension.

          Show
          Marc Marc added a comment - No, <!-- INCLUDECSS {T_THEME_PATH} /portal.css --> only looks inside prosilver and not the prosilver folder of the extension.

            People

            • Assignee:
              EXreaction EXreaction [X] (Inactive)
              Reporter:
              VSE Matt Friedman
            • Votes:
              0 Vote for this issue
              Watchers:
              5 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development