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

core.user_setup event doesn't support loading language files from extensions

    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: Events
    • Labels:
      None

      Description

      The core.user_setup event allows mods to add globally available language strings that may overwrite core language strings. Extensions, however, are not able to use this mechanism, as their language files need to be loaded differently. To allow mods to be converted to extensions as easily and cleanly as possible, the event should be adapted to allow loading extension language files as well.

        Activity

        Hide
        VSE Matt Friedman added a comment - - edited

        If this change is intended to load all extension lang files at user_setup, it will be problematic.

        Language files are already easily added to an extension by simply adding

        $user->add_lang_ext('foo/bar', 'foobar');

        to a function listening to core.user_setup in an event listener, or you can do it as needed inside your extension php controller files.

        This system work, and is better than your global auto-loading proposal for the following reason:

        I have extensions with gigantic ACP language files, and no needed lang files for outside the ACP. In addition, stand-alone controller files (like a blog ext) may only need their lang files when they are loaded, but never anywhere else in the forum. This PR would load these extra language files all the time, even though they may only be needed in the ACP or in the standalone file. Also, permission language files, which are separate files, are already being loaded too. So this PR would really just load everything, all the time, even when not needed, and prevent authors from controlling which of their lang files get loaded when/where they want, and the end result is just heaping on more system memory usage on an already bloated and memory heavy phpBB.

        Show
        VSE Matt Friedman added a comment - - edited If this change is intended to load all extension lang files at user_setup, it will be problematic. Language files are already easily added to an extension by simply adding $user->add_lang_ext('foo/bar', 'foobar'); to a function listening to core.user_setup in an event listener, or you can do it as needed inside your extension php controller files. This system work, and is better than your global auto-loading proposal for the following reason: I have extensions with gigantic ACP language files, and no needed lang files for outside the ACP. In addition, stand-alone controller files (like a blog ext) may only need their lang files when they are loaded, but never anywhere else in the forum. This PR would load these extra language files all the time, even though they may only be needed in the ACP or in the standalone file. Also, permission language files, which are separate files, are already being loaded too. So this PR would really just load everything, all the time, even when not needed, and prevent authors from controlling which of their lang files get loaded when/where they want, and the end result is just heaping on more system memory usage on an already bloated and memory heavy phpBB.
        Hide
        nickvergessen Joas Schilling added a comment -

        the problem with

        $user->add_lang_ext('foo/bar', 'foobar');

        Is, that extensions are unable to overwrite language variables from the core, if they need to.

        And if you'd look at the patch you'd see, that the patch does not add simply every file...

        Show
        nickvergessen Joas Schilling added a comment - the problem with $user->add_lang_ext('foo/bar', 'foobar'); Is, that extensions are unable to overwrite language variables from the core, if they need to. And if you'd look at the patch you'd see, that the patch does not add simply every file...
        Hide
        VSE Matt Friedman added a comment - - edited

        I couldn't tell from the patch if it loads everything or if there was some special name format or special location for these files (which should have been indicated somewhere if that is the case). But I get it now. :/

        Show
        VSE Matt Friedman added a comment - - edited I couldn't tell from the patch if it loads everything or if there was some special name format or special location for these files (which should have been indicated somewhere if that is the case). But I get it now. :/
        Hide
        nickvergessen Joas Schilling added a comment -

        The code is part of an event, so an extension must create a listener and loads what ever it wants to...

        Show
        nickvergessen Joas Schilling added a comment - The code is part of an event, so an extension must create a listener and loads what ever it wants to...

          People

          • Assignee:
            nickvergessen Joas Schilling
            Reporter:
            rechosen rechosen [X] (Inactive)
          • Votes:
            0 Vote for this issue
            Watchers:
            3 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development