Hook startup functions on on_page_view event instead of on_start

[hissy]

Setup translation language before startup/process.php.
Related commit: concretecms/concretecms@4fdea7e

It even works fine on before 5.6.3, but I want to more testing about this change by others.

[hissy]

Forum discussion: http://www.concrete5.org/marketplace/addons/internationalization/forums/internationalization-package-does-not-change-locale-of-core-bloc/

[mlocati]

What about tools? In dispatcher.php require($cdir . '/startup/tools.php'); comes before Events::fire('on_page_view', $c, $u);...

The source of the problem is that the localization must be initialized as soon as possible.
That's why I suggested concrete5#1375 + addon_internationalization#39

My solution would fix locale-dependent constants and setup the correct locale at a very early step of the dispatcher.

[hissy]

I totally agree with early initialize localization, but maybe we need more discussion about add warm up method... Anyway, this change will fix some problem in Internationalization after core 5.6.3 released.

[mlocati]

Yes, it fixes some problems by initializing multilingual a bit earlier (but as I stated above it should be initialized a lot earlier)

[mlocati]

Another problem with this approach is that package localizations does not get loaded:
setupPackageLocalization in called before the on_page_view event...

[Remo]

@mlocati package localizations get loaded after the last few changes you've made, don't they? The only problem left is the tools related issue? I believe this PR doesn't have any negative effects but fixes an important bug and should therefore be merged.

[mlocati]

Yes. Packages localization gets loaded at Localization initialization and when Localization::changeLocale is called.

[Remo]

@tylerryan Any chance this can get merged? I'd still prefer to have the whole i18n stuff in the core where we could initialize it even earlier, but this pull request fixes one rather annoying bug.

[Remo]

@mlocati @hissy I've got write access to this repo.. We've been using a custom version of this add-on with this PR applied. Any objections about merging it?

[mlocati]

If it works I can't see any reasons to not merge this 😉

[Remo]

it does work for me and as long as no one else complains I'm good, thanks (: