? J4 Issue ?
avatar SharkyKZ
SharkyKZ
22 Mar 2019

Steps to reproduce the issue

Enable System Debug in Global Configuration.
In System - Debug plugin Set Refresh Assets to No.
Open a page where multiple WebComponents are loaded.
Inspect source code.

Expected result

WebComponents have no media version appended, e.g.:

"webcomponents": [
        "\/400\/media\/vendor\/joomla-custom-elements\/js\/joomla-tab.js?",
        "\/400\/media\/system\/js\/fields\/joomla-field-switcher.js?",
        "\/400\/media\/system\/js\/joomla-toolbar-button.js?",
        "\/400\/media\/vendor\/joomla-custom-elements\/js\/joomla-alert.js?"
    ]

Actual result

Some have and only some don't, e.g.:

"webcomponents": [
        "\/400\/media\/vendor\/joomla-custom-elements\/js\/joomla-tab.js?62b28cbd90156e99ecbecb5f894fb3e5",
        "\/400\/media\/system\/js\/fields\/joomla-field-switcher.js?62b28cbd90156e99ecbecb5f894fb3e5",
        "\/400\/media\/system\/js\/joomla-toolbar-button.js?",
        "\/400\/media\/vendor\/joomla-custom-elements\/js\/joomla-alert.js?"
    ]

System information (as much as possible)

4.0-dev Nightly.

Additional comments

The example above happens because the Debug plugin uses onAfterDispatch event to set document's mediaVersion property. But component's buffer has already been set by that point so it has no effect on scripts added by the component. Either the document needs to be fetched earlier or we need another event before component is dispatched.

avatar SharkyKZ SharkyKZ - open - 22 Mar 2019
avatar joomla-cms-bot joomla-cms-bot - change - 22 Mar 2019
Labels Added: ?
avatar joomla-cms-bot joomla-cms-bot - labeled - 22 Mar 2019
avatar SharkyKZ SharkyKZ - change - 22 Mar 2019
The description was changed
avatar SharkyKZ SharkyKZ - edited - 22 Mar 2019
avatar franz-wohlkoenig franz-wohlkoenig - change - 22 Mar 2019
Status New Discussion
avatar franz-wohlkoenig franz-wohlkoenig - change - 22 Mar 2019
Category Code style
avatar mbabker
mbabker - comment - 22 Mar 2019

The Document object can only be instantiated after routing is done because it is format aware, which is why anything trying to load scripts in onAfterInitialise or onAfterRouting (because the Document isn't created before this event is emitted) is just dead wrong and will break things. So there is really not much choice in the matter as far as when it is created. I'd also be very hesitant to inject a onBeforeDispatch event 2/3's of the way through CMSApplication::dispatch(), either it needs to be at the beginning of the method (which is before the Document is created) or you create a new suite of on(Before|After)ComponentDispatch events and leave the system level onAfterDispatch event alone.

avatar SharkyKZ
SharkyKZ - comment - 23 Mar 2019

Pardon my ignorance, but are you saying it's a matter of naming convention? Can we place an event other than onBeforeDispatch midway through dispatch() (e.g. onAfterLoadDocument or whatever)?

onAfterComponentDispatch seems redundant as it would be triggered at the same point as onAfterDispatch.

avatar mbabker
mbabker - comment - 23 Mar 2019

I would not have a unique system event for the document being loaded. Why not one for the language, session, or any other service in Joomla\CMS\Factory?

Having two simultaneous events, while redundant, is not necessarily an issue if they are intended to be in differing contexts. Plus, if something changes where the last couple of lines of the application's dispatch method aren't the call to render the component and the onAfterDispatch event then said redundancy goes away. Would you remove an event call because you refactored something and suddenly two event calls that were separated by a few lines of code ended up next to each other?

TBH, I have always felt like the way the media version is appended to things is wrong. When adding a resource to the document, the version should be part of the attributes array and when the renderer creates the HTML representation THEN the version should be appended to the resource URI. The fact the version gets appended to the URI before the add(Script|Stylesheet) call is problematic for a lot of reasons, including this one you're trying to deal with.

avatar SharkyKZ
SharkyKZ - comment - 25 Mar 2019

Stylesheets and scripts don't have this issue, they indeed have version appended in the renderer. In core this currently affects only Joomla\CMS\HTML\HTMLHelper::webcomponent(). WebComponents are added using document's addScriptOptions() so setting media version in the renderer is not possible. I don't see why would it be wrong to use document's properties within HTMLHelper methods. What if one method needed to have different result based on document properties. Maybe irrelevant at this point, but, for example, different markup for HTML5 and non-HTML5 pages? Really the issue is that document is populated before plugins can modify its properties.

So adding on(Before|After)ComponentDispatch events is the way to go then?

avatar mbabker
mbabker - comment - 25 Mar 2019

So adding on(Before|After)ComponentDispatch events is the way to go then?

That's how I'd do it. I honestly think your issue here has more to do with the state of the architecture than really needing a dedicated onAfterDocumentCreatedButBeforeComponentDispatched hook point and the new hook point is just the quick fix to satisfy a short term issue. And actually, it seems you've already nailed it on the head.

In core this currently affects only Joomla\CMS\HTML\HTMLHelper::webcomponent().

If web components are being added by this method in a way where the version hash is immutable, even though other assets have a mutable hash until everything is rendered, then there's an issue with how webcomponents are registered, not the lack of a hook point between creating the document and dispatching the component.

avatar SharkyKZ
SharkyKZ - comment - 26 Mar 2019

It's just not about media version or web components though. This is about whether setting any of document's properties in plugins and using them outside of document and its renderers is valid or not. I would say that it is.

Making web components a document property and moving them to a renderer would be the short term solution as it wouldn't solve potential similar problems elsewhere.

avatar mbabker
mbabker - comment - 26 Mar 2019

Aside from dealing with components that early exit because plugins have a knack for breaking things, name something that a plugin cannot alter about a Document instance at onAfterDispatch or any hook point after that that it would be able to with a hook before the component is executed. My point is if you need a hook in that specific spot, it seems like some other integration is messed up and that hook is just a workaround to fix that issue. We've identified the webcomponent issue. If that's the only thing then the webcomponent issue should be fixed, how it's fixed I couldn't tell you (I don't think it warrants its own API in the root Document class, MAYBE I can be convinced having it as part of only the HtmlDocument is acceptable, but if I'm also being blunt the Document class is something that needs to be broken apart and deprecated).

avatar SharkyKZ
SharkyKZ - comment - 26 Mar 2019

Web components is currently the only exposed issue in the core. But 3rd party plugins would expose more issues wherever documents properties are used in component. For example, if a plugin was to set document's direction property, this part of Web Installer would fail:

$this->rtl = strtolower(Factory::getDocument()->getDirection()) === 'rtl' ? 1 : 0;

avatar mbabker
mbabker - comment - 26 Mar 2019

I'd say that specific case makes the point that plugins should be restricted from doing some things, not being able to do more ?

avatar SharkyKZ
SharkyKZ - comment - 27 Mar 2019

?

Which things? Setting document properties, for example?

avatar mbabker
mbabker - comment - 27 Mar 2019

That was a bad example to highlight because if a plugin is changing the direction property of the document in an unexpected way it's probably going to break things for the rest of that request in a bad way, especially if you're in a template that has RTL support.

My main point here is I don't see why the document needs a special event after it is created to allow plugins to do things before the component is dispatched. And if the document is this special, then why not the session, or the language, or the DI container, or the user, or the database driver, or the mailer (granted, half of those you couldn't dispatch events for because of dependency hell with the CMS architecture but that's beside the point). If you really think the document needs this type of hook, I would honestly say move its creation to before the onAfterRoute event is dispatched but even that is architecturally wrong because CMSApplication::route() should not have the responsibility of creating that object.

Otherwise, your best least hacky solution is the component dispatch events I suggested before.

avatar SharkyKZ
SharkyKZ - comment - 31 Mar 2019

if a plugin is changing the direction property of the document in an unexpected way

What if it's doing it in a totally expected way?

avatar SharkyKZ
SharkyKZ - comment - 31 Mar 2019

This has completely slipped my mind until now but the issue also occurs when properties are being set in the template (e.g. HTML5 mode in J3.x). Because component is rendered before the template is even parsed.

avatar mbabker
mbabker - comment - 31 Mar 2019

On the language case, name a valid reason for a plugin to flip the
direction from that set by the language for the entire document.

What can a component render that should be different based on HTML4 versus
HTML5?

All these hypotheticals can run around in circles all day. The document
can be in a different state between when a component is rendered and when
the template is rendered, there’s no getting around that without a major
core rewrite where the component is dispatched as part of the document
renderer. Even that doesn’t block the possibility of modules changing the
document before a plugin changes more stuff in the document.

Face it, the document API design is broken because of the core
request-response workflow. Adding plugin events doesn’t fix anything. Fix
the broken webcomponent processor, don’t try to monkey patch core.

On Sun, Mar 31, 2019 at 3:43 PM SharkyKZ notifications@github.com wrote:

This has completely slipped my mind until now but the issue also occurs
when properties are being set in the template (e.g. HTML5 mode in J3.x).
Because component is rendered before the template is even parsed.


You are receiving this because you commented.
Reply to this email directly, view it on GitHub
#24308 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAWfoXpp6TaM964avJpTLCPTB91eTQ7Zks5vcR4PgaJpZM4cDHXI
.

--

  • Michael Please pardon any errors, this message was sent from my iPhone.
avatar franz-wohlkoenig franz-wohlkoenig - change - 4 Apr 2019
Labels Added: J4 Issue
avatar franz-wohlkoenig franz-wohlkoenig - labeled - 4 Apr 2019
avatar franz-wohlkoenig franz-wohlkoenig - change - 7 Apr 2019
Labels Added: ?
avatar franz-wohlkoenig franz-wohlkoenig - labeled - 7 Apr 2019
avatar franz-wohlkoenig franz-wohlkoenig - change - 9 Apr 2019
Category Code style
avatar SharkyKZ
SharkyKZ - comment - 10 Apr 2019

Yes, a plugin event alone won't fix the issue, that's what I meant with my previous comment. The issue with setting document properties in the template would remain.

How architecturally invalid would it be to split CMSApplication::render() into parse() and render() and move dispatch() in-between? This would sort of move the component rendering closer to other parts of the document.

avatar mbabker
mbabker - comment - 10 Apr 2019

What is the application parsing? It definitely isn't parsing a template, truthfully it's already wrong that the application class is responsible for rendering the template, in a MVC structured application that is the role of your controller and view (where the full application is MVC, whereas the rendering bits of the CMS architecture (components and modules) really are still executed in a procedural non-OOP manner just wrapped into classes, the fact most components switch to MVC once you get to components/com_content/content.php doesn't really matter because aside from shoving some files in the right places to integrate with things like com_config or com_menus nothing says a component must use a MVC architecture).

Architecturally, there is not much I would do to change how the CMS runs. There is just too much code floating around out there expecting the CMS to execute in the way it has basically since 1.5's release 11 years ago. To change the architecture to "fix" the fact the Document can be manipulated and can (and will) be in a different state between when the component is actually dispatched to when the template file is parsed to when each document renderer is actually executed essentially means you would have to change everything so that once the document starts printing HTML it must go into a read-only state (otherwise you never address the "what if a plugin changes the direction between X and Y renderers" or "what if the doc is switched from HTML4 to HTML5 after component is dispatched" scenarios). Which means changing the entire request-response workflow in core. That's probably not happening.

avatar SharkyKZ
SharkyKZ - comment - 15 Apr 2019

what if the doc is switched from HTML4 to HTML5 after component is dispatched

The doc should not switch from HTML4 to HTML5 after the component has been dispatched. But it does exactly that because the document is parsed only after the component has already been dispatched.

what if a plugin changes the direction between X and Y renderers

Again, my point is that changing document properties that affect rendering should only be considered valid if nothing has been rendered yet. At the moment this is not possible because the component is rendered as soon as the document is created. There is no valid plugin event for changing document properties.

avatar mbabker
mbabker - comment - 15 Apr 2019

Then honestly you're going to have to introduce a major rewrite of the core request-response cycle. Because there is just no way to force the Document API into an immutable state with the current request workflow, or the fact that the entire Document API is public properties. The fact of the matter is you cannot achieve what it is you are looking for with the core architecture in the state it is in and to get to the state you are looking for is a massive undertaking because you're basically rewriting how Joomla works between the point routing is finished and the point the response is sent to the browser.

avatar mbabker
mbabker - comment - 15 Apr 2019

The doc should not switch from HTML4 to HTML5 after the component has been dispatched.

And, I have to point this one out separately. The Document API knows nothing about rendering components. As I'm sure you've already figured out, the rendered component is pushed into the Document API rather than the Document API being responsible for rendering that part of the page. So how does "changing document properties that affect rendering should only be considered valid if nothing has been rendered yet" apply when the API doesn't know if something has been rendered?

There is no valid plugin event for changing document properties.

Nor, should there be. Just focusing in on public properties available in the HtmlDocument (and obviously parent Document), excluding underscored properties because those should've been made protected ages ago but screw proper API design:

  • baseurl: If someone changes this, they deserve a broken page
  • params: Yay for undocumented arrays, what's supposed to be here, and isn't that a Registry object anyway?
  • title: Why does this need to be set before the component is dispatched, isn't the component responsible for setting this?
  • description: Why does this need to be set before the component is dispatched, isn't the component responsible for setting this?
  • link: Restricting changing this would break SEO plugins, no?
  • base: No idea where it's used off hand, but it seems like if someone's changing this they deserve what they get
  • language: Really, if anyone's changing this after the document is instantiated they're a jerk, respect the request's language that has already been determined for you
  • direction: Same as above

Omitted here is the HtmlDocument's HTML5 flag. So now I ask, why should a component behave differently based on whether the template is HTML4 or HTML5 or XHTML or whatever the next generation thing is? Same for a module. The default template files at the extension level are not designed to be one size fits all, if you REALLY want to force a component's output to be 100% HTML4 compliant and not use <main>, <nav>, <article>, etc. (which is polyfilled well enough that it doesn't even matter anymore) then use the override system and change everything to <div>. I don't think it's component or module provider responsibility to change their output based on that flag in the template otherwise you are asking for a lot of new problems to be pushed onto the extension vendors.

avatar SharkyKZ
SharkyKZ - comment - 16 Apr 2019

changing document properties that affect rendering should only be considered valid if nothing has been rendered yet

To clarify, by this I literally meant what developers should consider to be a valid point for setting some of the properties. And at the moment, such point simply does not exist. So I am not saying we should restrict setting anything. It should obvious that setting document properties during onAfterRender or onAfterRespond events is not valid and such cases don't need to be accounted for. Even having $this->setHtml5(true) in the template is not valid for the same reason.

Omitted here is the HtmlDocument's HTML5 flag. So now I ask, why should a component behave differently based on whether the template is HTML4 or HTML5 or XHTML or whatever the next generation thing is?

I don't know. Why does HtmlDocument renderers output different markup based on this? Why does this property even exist then? But this is really besides the point. It's not about one property or another.

avatar mbabker
mbabker - comment - 16 Apr 2019

To clarify, by this I literally meant what developers should consider to be a valid point for setting some of the properties.

What properties need to be set before a component is dispatched that would affect how a component is dispatched? Given the way components are supposed to be decoupled from templates in everything but layouts, there should not be anything here that one could theoretically change that affects a component (and the properties that might are the language and direction and as I said if you're changing those away from the request language and direction you're a jerk). What makes the Document object so special that it must have an event emitted for its creation in relation to other system objects?

Why does HtmlDocument renderers output different markup based on this?

It's the only way to tune the <head> renderer to output <link> and <script> tags in HTML4 or HTML5 compliant syntax without hardcoding it one way or the other (i.e. core shouldn't be hardcoded so that these tags can only be printed in HTML4 compliant syntax since this is something dictated by oldest supported browser and IIRC J3 supports browsers which have no idea what HTML5 is). Other than that, it has no real use throughout the ecosystem that I'm aware of.

avatar SharkyKZ
SharkyKZ - comment - 17 Apr 2019

Any properties that should be consistent throughout the document. We really just need to go back to the question of whether or not these properties should be usable outside of document renderers. What do you think? Would it be wrong to check HTML version in, for example, HTMLHelper methods?

avatar mbabker
mbabker - comment - 17 Apr 2019

Any properties that should be consistent throughout the document.

To accomplish this means rewriting the core request-response workflow. A plugin event on a mutable property before the component is dispatched, which is still massively before the template file is parsed for <jdoc> tags and executed to actually do its PHP stuff doesn't fix anything; if anything it potentially makes things worse.

Would it be wrong to check HTML version in, for example, HTMLHelper methods?

No, but aside from the inline script that the email cloaking helper generates, what actually should have a different behavior based on HTML4 or HTML5?

avatar SharkyKZ
SharkyKZ - comment - 19 Apr 2019

It doesn't have to be mutable. Anyone who tries changing it after a buffer has been set is a jerk anyways ? .

Switching up dispatch/parse/render with parse/dispatch/render does seemingly fix both of the issues I had in mind. Though not sure what it breaks yet. Component assets are added after template's, for one. But the same issues already exists with modules and plugins. Any chance of having some template helper class in core for pre/post-processing templates? So their assets could be added in correct order?

avatar franz-wohlkoenig
franz-wohlkoenig - comment - 16 May 2019

@SharkyKZ can you please sens an E-Mail to franz.wohlkoenig@community.joomla.org? I have to ask you and didn't find another Way for it.

avatar SharkyKZ SharkyKZ - change - 25 Mar 2021
Status Discussion Closed
Closed_Date 0000-00-00 00:00:00 2021-03-25 17:41:37
Closed_By SharkyKZ
avatar SharkyKZ SharkyKZ - close - 25 Mar 2021

Add a Comment

Login with GitHub to post a comment