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

ACP Template files not purged during Extension Enable/Disable

    Details

    • Type: Bug
    • Status: Unverified Fix
    • Priority: Blocker
    • Resolution: Fixed
    • Affects Version/s: 3.1.0-dev, 3.1.0-a2
    • Fix Version/s: 3.1.0-a3
    • Component/s: ACP, Extensions
    • Labels:
      None

      Description

      It appears that not all template files are being purged from the cache during the Enable and Disable processes of an extension. It appears the ACP template files do not get purged.

      However they are all purged when choosing the Delete Data option during the Disable extension process.

      As an example, I have an extension that adds an external JS file to the ACP's overall_header file via template event. Upon Enabling the extension, the cache must manually be purged in order to get the extension's JS feature in the ACP working.

        Activity

        Hide
        VSE Matt Friedman added a comment - - edited

        Attached at top is a simple extension designed to test this bug. Test enabling and disabling of this extension with DEBUG mode off.

        It just has a template event for the ACP overall_header.html that adds "Hello World" to the top of the ACP pages.

        When you enable the ext, you should immediately see "Hello World". But in my tests, it takes a manual purge of the cache to get it to appear. If you disable it, you get a fatal error, and an error like:

        PHP Fatal error: Uncaught exception 'Twig_Error_Loader' with message 'There are no registered paths for namespace "acme_bugtest" in "overall_header.html" at line 107.

        Show
        VSE Matt Friedman added a comment - - edited Attached at top is a simple extension designed to test this bug. Test enabling and disabling of this extension with DEBUG mode off. It just has a template event for the ACP overall_header.html that adds "Hello World" to the top of the ACP pages. When you enable the ext, you should immediately see "Hello World". But in my tests, it takes a manual purge of the cache to get it to appear. If you disable it, you get a fatal error, and an error like: PHP Fatal error: Uncaught exception 'Twig_Error_Loader' with message 'There are no registered paths for namespace "acme_bugtest" in "overall_header.html" at line 107.
        Hide
        VSE Matt Friedman added a comment - - edited

        After more testing, here is a better way to describe the issue of this bug.

        The cache is being fully purged. So the issue is not really cache purging. The issue is that immediately following the cache purge during an extension being enabled or disabled, the newly generated cache files for specifically the acp_overall_header/footer.html files do not contain the newly enabled extension (or they still contain the just disabled extension).

        So the issue is the ACP overall_header/footer are not being updated properly to reflect the enabled/disabled state of an extension during the enable/disable processes.

        This is fatal for extensions that want to add any template events to the header or footer of the ACP.

        But this goes for all extensions. Disable any extension, no matter what it does. You will see that there are stale references to it in the ACP overall_header/footer in the form of

         if ($this->env->getLoader()->exists('@acme_bugtest/event/acp_overall_header_head_append.html'))

        I guess you could say its the ACP overall_header/footer template events that are not aware of the correct state of an extension after enabling/disabling.

        Show
        VSE Matt Friedman added a comment - - edited After more testing, here is a better way to describe the issue of this bug. The cache is being fully purged. So the issue is not really cache purging. The issue is that immediately following the cache purge during an extension being enabled or disabled, the newly generated cache files for specifically the acp_overall_header/footer.html files do not contain the newly enabled extension (or they still contain the just disabled extension). So the issue is the ACP overall_header/footer are not being updated properly to reflect the enabled/disabled state of an extension during the enable/disable processes. This is fatal for extensions that want to add any template events to the header or footer of the ACP. But this goes for all extensions. Disable any extension, no matter what it does. You will see that there are stale references to it in the ACP overall_header/footer in the form of if ($this->env->getLoader()->exists('@acme_bugtest/event/acp_overall_header_head_append.html')) I guess you could say its the ACP overall_header/footer template events that are not aware of the correct state of an extension after enabling/disabling.
        Hide
        nickvergessen Joas Schilling added a comment -

        The problem is the order when creating the page:
        1. page started
        2. template class is initialised with "$this->extension_manager->all_enabled()"
        3. in acp the command disable is called for the abc extension
        4. template class still has abc extension in its array
        5. the template code uses this array for its code generation and uses the disabled extension

        Show
        nickvergessen Joas Schilling added a comment - The problem is the order when creating the page: 1. page started 2. template class is initialised with "$this->extension_manager->all_enabled()" 3. in acp the command disable is called for the abc extension 4. template class still has abc extension in its array 5. the template code uses this array for its code generation and uses the disabled extension
        Hide
        EXreaction EXreaction [X] (Inactive) added a comment -

        This should be an easy fix I think.

        Change the twig\environment constructor to accept the extension manager class rather than just the list of extensions at initialization. Then in the twig\environment\get_phpbb_extensions() function return all_enabled() (by the time this is called, all_enabled() should have the correct info).

        Show
        EXreaction EXreaction [X] (Inactive) added a comment - This should be an easy fix I think. Change the twig\environment constructor to accept the extension manager class rather than just the list of extensions at initialization. Then in the twig\environment\get_phpbb_extensions() function return all_enabled() (by the time this is called, all_enabled() should have the correct info).
        Hide
        VSE Matt Friedman added a comment -

        PR submitted

        Show
        VSE Matt Friedman added a comment - PR submitted

          People

          • Assignee:
            nickvergessen Joas Schilling
            Reporter:
            VSE Matt Friedman
          • Votes:
            0 Vote for this issue
            Watchers:
            4 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development