Override a core class within a package

Permalink 2 users found helpful
I'm trying to override a core class libraries/view.php from a package but it doesn't seem to be working at all. Can someone please tell me what i'm doing wrong?

Here's the code in my package/controller.php on_start:
public function on_start(){
      $objPkg = Loader::package($this->pkgHandle);
      $objEnv = Environment::get();
      $objEnv->overrideCoreByPackage('libraries/view.php', $objPkg);
   }


i've also copied the libraries/view.php into my package folder and have a die() statement in the getThemePath just to see if it's working:

public function getThemePath()
    {
        die('wtf??');
    }


It never dies... What gives?

Please note that i've turned off override cache and i've cleared cache as well, many times.

Thanks in advance for any help.

thronic
 
jordanlev replied on at Permalink Reply
jordanlev
I don't think Loader:package() is a thing. Try just passing $this to the overrideCoreByPackage() function (since you're already in the package's controller, $this should refer to the package).
thronic replied on at Permalink Reply
thronic
well, here's some more to make you (well me really) scratch you head. I decided to try to override a helper class as a test:

public function on_start(){
      $objPkg = Loader::package($this->pkgHandle);
      $objEnv = Environment::get();
      $objEnv->overrideCoreByPackage('libraries/view.php', $objPkg);
      $objEnv->overrideCoreByPackage('helpers/html.php', $objPkg);
   }


And sure enough it dies out when i extend one of it's methods!

To be sure i commented out this line:

//$objEnv->overrideCoreByPackage('helpers/html.php', $objPkg);


And upon reload it no longer dies out.

So now i'm very sure i'm doing it correctly but it just doesn't seem to work with the libraries/view.php. Is this a bug or a known issue or are you is there something further that needs to be done to extend a "libraries" class.

Thanks for responding, BTW. ;)
jordanlev replied on at Permalink Reply
jordanlev
I honestly didn't know about the overrideCoreByPackage thing. You might want to check and make sure your custom libraries/view.php file is declared like this:
class View extends Concrete5_Library_View {}

...and *NOT* like this:
class Concrete5_Library_View extends Object {
thronic replied on at Permalink Reply
thronic
yes it is:

class View extends Concrete5_Library_View {
thronic replied on at Permalink Reply
thronic
and still no go...
jordanlev replied on at Permalink Reply
jordanlev
Sorry, I have no clue :(
Hopefully someone else who has more knowledge about this can chime in.
beebs93 replied on at Permalink Best Answer Reply
beebs93
Long story short: You cannot do it with a package.

Some classes can be overridden via a package like the example you used, but not ALL of them.

The Package controller's on_start() method doesn't fire as early as you may think and essential classes, like for page rendering/caching, are needed before the the list of overrides by packages has been built. Thus, concrete5 doesn't know to use your version of the file (nor will it).

You'll have to override the file the old way by putting the view.php in the SITE_ROOT/libraries directory and modify whatever methods you'd like.
dpatterson replied on at Permalink Reply
Okay. I've been working this same thing for the past day or two.
Finally figured it out.

Beebs93 is essentially correct. Specifically, dispatcher.php loads /config/theme_paths.php _before_ it initializes the packages. Since theme_paths.php instantiates a View object, the auto loader loads the default View class preventing the package-specific class from ever even being checked for.

As a test, I moved the
require($cdir . '/config/theme_paths.php');
line down to just after the
require($cdir . '/startup/packages.php');
line in /concrete/dispatcher.php.
It worked! And, in testing so far, I have seen no ill effects.

You do need to use the overrideCoreByPackage() method, though. Just placing view.php in the package's libraries directory won't work.
$env = Environment::get();
$env->overrideCoreByPackage( 'libraries/view.php', $this );


I think, maybe, that I'll post a bug/feature request about this.

Hope this helps.
D.
JohntheFish replied on at Permalink Reply
JohntheFish
I think this would make a good 'advanced' howto if you can spare the time to write it up.
dpatterson replied on at Permalink Reply
Hmm. A how-to on which, how to override the default View class or how I figured out what was going on? :-)
JohntheFish replied on at Permalink Reply
JohntheFish
About overriding the technique for overriding the view by tweaking the dispatcher code and then overriding the dispatcher.

I have a suspicion that similar techniques may be appropriate for things like getting 5.6.1+ to process package on_start events when the cache is fully enabled.

It would definitely be an advanced developer level howto. Not something written to talk beginners through it.
dpatterson replied on at Permalink Reply
Once I actually get my custom view stuff working, I'll give some thought to writing up a how-to.
beebs93 replied on at Permalink Reply
beebs93
So dumb question:

Wouldn't future concrete5 updates wipe out any changes to the /concrete/dispatcher.php file?
JohntheFish replied on at Permalink Reply
JohntheFish
override
dpatterson replied on at Permalink Reply
Yep. That's why I filed a bug report.
I've been working with this all afternoon and still haven't seen any adverse side effects from the change.
dpatterson replied on at Permalink Reply
JohntheFish, maybe you'll have some input on a related issue.
The reason that I need to override the default View class is because I want to build a clean way to use Smarty when rendering for a custom theme.

I really don't want to get into the whole Smarty vs imbedded PHP argument. Everybody is entitled to their own opinion and I happen to like using Smarty.

So, here is what I had in mind: Extend the Concrete5_Library_View{} class replacing the render() method with one that knows about Smarty. The goal is to add Smarty support without breaking anything else.

What I had hoped to do was check the theme from within render() and call an appropriate protected method accordingly (renderWithSmarty() or renderAsUsual(), for instance).

What I'm running into, though, is that I haven't been able to come up with a reliable method for render to determine whether the theme is using Smarty. The way the PageTheme object is rendered seems to preclude extending that class (it's constructed and then filled separately).

Ideas?

TIA
JohntheFish replied on at Permalink Reply
JohntheFish
I have not worked with smarty, so my thoughts below are interpolated from my use of various template engines from Perl.

Is there a reason why you don't build smarty into the theme itself, so you had a smarty enabled area within the theme? or do you want a general purpose smarty renderer that will process any smarty enabled theme into one that c5 will then process the usual way?

If the later, an on_start event handler in a package controller used to be a good place to do this sort of thing. But with the latest c5 it doesn't fire when full page caching is enabled.

Hence my interest in what you are doing, as I have lots of uses for that event and this whole issue feels like it stems from a similar root.

The last couple of days I have been adding sources to my universal content puller addon. Maybe a smarty content source for something like that would be feasible?

Another possibility is to render the template in the browser using one of the jQuery based template engines.
dpatterson replied on at Permalink Reply
Thanks for the input.

Actually, the long-term goal is to be able to use Smarty to render anything from a block to an entire page without breaking all of the existing themes and blocks.

What I was thinking for themes was a flag related to the theme that indicates it uses Smarty and then render accordingly within the View::render() method. Unfortunately, there doesn't seem to be anywhere to hook that. The View::render() method actually fires an on_start event that I thought I might be able to grab in the PageTheme object, but, alas, the event is fired before the PageTheme object is instantiated.

Right now I'm trying to determine if I can realistically reorder things in the render() method and get the PageTheme instantiated before firing the event. Of course, I'm not entirely sure that will actually do be any good in the end.

I haven't thought blocks through that far yet.

I really prefer to do the template stuff on the server side. I've been working with Smarty for oh, 10 or 12 years, I guess and don't really have any desire to switch to another template engine. It works and works well.
thronic replied on at Permalink Reply
thronic
I'd love to know what they'll do with your bug request.

I've since worked around this by capturing the on_page_output event and doing what i needed to do. It would be much "nicer" if i didn't have to do it and i could override the view class.

My add-on is in the final stages of the peer review and should be available soon. Look for it: Eleven17 CDN.

Thanks!
dpatterson replied on at Permalink Reply
I haven't really spent any time with the C5 bug tracker, but it might help if others give the bug a thumbs up. :-)
JohntheFish replied on at Permalink Reply
JohntheFish
In case it has relevance to your work in progress: Yesterday I learned that addon packages are not supposed to use overrideCoreByPackage(). As a consequence I have made some minor changes to an addon I have in the PRB, so that users have to copy some files to root level overrides rather than rely on the addon to make the overrides directly.
dpatterson replied on at Permalink Reply
That just seems crazy to me.
From my point of view, the whole point of a CMS is letting the owner of the site maintain the site with virtually no tech knowledge.

I certainly don't want my artist friend who has no knowledge of the internal workings or directory layout of his website copying files around.

In fact, in my current installations the only access they have to the file system is to their uploads directories via SFTP.

In my, not so humble, opinion, Someone like that should be able to download, install, configure, and take advantage of any package in the marketplace with zero technical knowledge and zero access to the file system or the command line. It should be done through the CMS and file manipulations should be done automagically. That, of course, becomes difficult due to permissions if you're serious about site security. I don't let the HTTP server write to application directories.

Just saying...
JohntheFish replied on at Permalink Reply
JohntheFish
The reason given is that once a package uses overrideCoreByPackage(), there is no obvious trace left like there is with copied overrides and no means of prioritizing which package overrides what when multiple packages try to do it, so a concern that a site could consequently descend into chaos.

A question was asked about overrideCoreByPackage() on Totally Random a while back and the reply from Andrew was that PRB submissions using this functionality would be examined very closely. From the comment in the PRB, the official view has now changed and rather than adjudicate individual cases, its a blanket ban.

While frustrated, I appreciate the administrative problem unconstrained use would present (its why the question was asked in the first place). Fortunately, I had already planned for the 5.4/5.5 equivalent installation, so the changes to my code and documentation were minimal (just stripping a few lines out that made the 5.6 version more friendly)

There is nothing to stop us using it on site specific packages. Its just not going to be approved for anything for the marketplace.

The remaining mystery is 'So why is it called overrideCoreByPackage() if packages are not allowed to use it?'
hissy replied on at Permalink Reply
hissy
Thanks JohntheFish, it's very helpful answer for me.

We forked concrete5 4 years ago, and keep maintaining Japanese distribution now, because concrete5 has many problem to use with multibyte (non-alphabet) language. We need to use multibyte page name, multibyte file name, multibyte set name. However, I think distribution is not a better way.

So, I am trying to create multibyte enhancement package (it overrides some core helpers) in recent months, to share our code with international concrete5 users. In the future, we want to stop maintainance of our distribution. It is like Japanese WordPress users created Multilingual Edition in past, then they made WP Multibyte Patch, and stopped maintainance of WordPress ME.

However, PRB do not accept that using coreOverrideByPackage in marketplace package... I lost hope... So I have same question. Why is it called overrideCoreByPackage() if packages are not allowed to use it? And hopefully, I want to use it.
thronic replied on at Permalink Reply
thronic
Apparently, you can't even chain events either. If one add-on uses an event and alters something the next matching event that fires (from another add on) will over ride the changes that the initial event did.

So essentially you want you add on to run last... or just hope that you're the only add-on using that event.

See my post here:

http://www.concrete5.org/community/forums/customizing_c5/multiple-p...

Any ideas?