π is exactly three! And now that I have your attention, we're seriously considering removing [pagesetup, system] in 3.0.
It causes numerous problems due to firing at an very unpredictable time. We've already made changes to 2.0 to make it closer to 1.x timing, but pain persists. We're generally thinking pagesetup handler code should be moved to 1 of 3 different locations:
Are there any case these don't cover?
info@elgg.org
Security issues should be reported to security@elgg.org!
©2014 the Elgg Foundation
Elgg is a registered trademark of Thematic Networks.
Cover image by RaĆ¼l Utrera is used under Creative Commons license.
Icons by Flaticon and FontAwesome.
My gut feeling is that I don't want it to go away. I use it mostly for times when I need to make a decision before the page renders, once the page owner and context has been determined.
An example being: a plugin that protects pages from being viewed by unwanted users (a user-block plugin)
on pagesetup you can check who is viewing the page, and who the page belongs to, and then continue with the page generation or not. 'init', 'system' is too early in the process the page owner isn't set. Menu registration doesn't really fit with the purpose, and 'shell', 'page' can work for that purpose but you've already gone through the trouble of generating/rendering the content at that point, that's inefficient and kills TTFB when you're generating and throwing away pages that are never viewed.
Most of the way it is used can certainly be moved to the stated hooks/events, but I think it is still a useful event to have to mark the start of content generation. Maybe we just discourage its use where not appropriate?
Well, now imagine how you would solve the same problem in a different viewtype. Would you rely on page owner being set for a json resource? IMO, we need to make better use of page_owner hook and rely more on URL petterns. I would go as far as forbidding setting a page owner explicitly, that would force devs to use full URLs for partial resources and resolve page owner before view system kicks in.
What I am saying is that pagesetup gives too much levy for altering global state. It creates discrepancies when you work with SEO URLs, for example, or use route hooks heavily. I'd much rather plugins announce globally what page owner and context a certain URL represents, rather than those values being populated arbitrarily within views.
As a side note, as of late, I avoid sniffing for page owner and instead pass entities explicitly between views and hooks. It allows me to recycle views for seemingly unrelated purposes, safely use hooks in RESTful queries etc. This is especially relevant for menus, and will be a major issue if we transition to a javascript framework.
Usually. In 1.x there was an assumption that the page handler would set up page owner and context, then the first view would be rendered, then menus and page rendered, etc. This is a fragile setup that plugins can accidentally break easily.
We do not use pagesetup that much anymore. I have checked in what cases we still use the pagesetup event and what the alternative would be. This is what came up:
The alternative is to use the system_ready event or register:menu hooks or a new register:widgets hook (https://github.com/Elgg/Elgg/issues/9496)
The rest is more menu manipulation, but register:menu hooks can help with that.
So there are no real reasons that require me to have pagesetup
For widgets, I suggest hooking into view_vars for widget layout view. Similar with tools - before the form.
That is an option, but using the new propose hooks (which i am working on) it is even better, as group tool option availability and available widgets are also use in actions and before any view is executed.
Nonetheless... i still hear no objection for removing the pagesetup, other than Matt's gut feeling :)
If we keep punching Matt'a gut enough, maybe the feeling will become a PR :) I think what we need is a way to prevent views (and extensions) from being rendered via a hook (potentionally view_vars returning false)
- Previous
- 1
- 2
- Next
You must log in to post replies.