this makes total sense, but will conflict with my html5 PRs

https://github.com/joomla/joomla-cms/pulls?utf8=%E2%9C%93&q=is%3Aopen+is%3Apr+author%3Aandrepereiradasilva++html5

so, if ok with you, i will incorporate your $doc to $this changes there too.

I don't care either way as long as the changes happen.

I have tested this item ✅ successfully on c112e09

I have tested this item ✅ successfully on c112e09

RTC

I think this needs to be documented, if someone uses $doc in an overwrite it will fail or am I missing something?

No. Basically, there isn't a need to use $doc = JFactory::getDocument() in a template because the template is being rendered from the same JDocument instance that the factory has stored. And if it isn't for whatever reason (it's conceivably possible given the API structure, why it would happen beats the crap out of me), then it's possible to pull the wrong data if the JDocumentHtml instance that's rendering the template has different options than whatever JDocument instance is in the factory. So it's more correct to use $this in the templates to ensure they're pulling options from the JDocument instance that's rendering them.

I have understand the issue, but if someone has used $doc in an overwrite without declaring it, it will end up in a WSOD. That's too much pain for a patch release.

This doesn't affect layout overrides in any way. Layouts are rendered in scope of JViewLegacy, JModuleHelper, JLayoutFile, or a plugin class. It'd be an issue if we were including or requiring a file from within the template file and it depended on the declared variables from the template (for example the jsstrings.php file of Beez3 would be affected since it's included within the template).

@mbabker so what do you think 3.6.3 or 3.7.0?

It's fine for 3.6.3 as long as this doesn't conflict with anything @andrepereiradasilva has done (as he pointed out earlier, hence the reason I've not updated this PR at all).

ok, so I'll let it as it is and we are waiting what is with the changes @andrepereiradasilva is doing/has done

We've merged Andre's changes for HTML5 compat. So this needs a rebase now

This is rebased.

Merged with 09fe795

Have you tested error pages, too?

Yes. Error pages are in scope of a JDocumentError instance.

As $this is an instance of JDocumentError which extends JDocument, it's getBuffer method doesn't receive any parameters (contrary to JDocumentHtml).

I can't test the PR ATM, my patchtester just broke testing site

M'eh. This is why having subclasses with different signatures (which is an E_STRICT error) is bad.

What about JDocumentError extending JDocumentHtml, what is the secret behind it that this hasn't been done yet?

Personally, I'd rather deprecate and dump JDocumentError completely. Part of the problem behind it is it basically assumes all error pages are HTML.

If George ever publishes the 4.0 branch he's got going in his repo, I've got a refactored error rendering layer that addresses this notion which goes with my idea of deprecating and dumping JDocumentError. wilsonge@c48b817

The one detail not sorted out there is replacing the $this->error references that are available in the error.php layout files (which is why the HTML aware renderer still uses JDocumentError), but in the grand scheme of things, there's time to figure out a "good" replacement for that.

Yes, I'd be for deprecating it but as you say we're not having any replacement ATM.

An inspiration could be Slim framework uniform error handler that's trying to handle common document formats (html, xml and json).

Or every JDocumentXxxx class could have handleError method for handling errors.

Anyway I was able to test the PR and it fails, as error page doesn't show any modules but it's supposed to show at least menu and search module.

Not a lot of free time ATM but replace the getBuffer calls with:

<?php foreach (JModuleHelper::getModules('position-whatever') as $module) :
    echo JModuleHelper::renderModule($module, ['style' => 'whatever']);
endforeach; ?>

BTW: Looks like status codes are translated in v3.4.8 and v3.6, I see it for the first time.
Status code of 404 page with Polish as a default language is 404 Artyku%C5%82u nie znaleziono which is broken UTF8 as status messages are ASCII-only

The status code is using the Exception object's message, not a HTTP
standard message. So that's why.

On Wednesday, September 7, 2016, Piotr notifications@github.com wrote:

BTW: Looks like status codes are translated in v3.4.8 and v3.6, I see it
for the first time.
Status code of 404 page with Polish as a default language is 404
Artyku%C5%82u nie znaleziono.

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