I have tested this item ? unsuccessfully on 64d9bb2

1. Set Time-Zone to "Berlin",
2. applied PR,
3. change Category > +2h in Calendar-Field
4. changed back to 1st Category > additional 2h (4 Hours)

Do the publishing dates also shift?

I have tested this item ✅ successfully on 64d9bb2

Test again, all works fine now.

We need to be careful of ACL here - this is going to expose public data if accessed directly from a URL

Really, it uses the same code and checks as the save function. Which line do you think can be dangerous?

Really, it uses the same code and checks as the save function. Which line do you think can be dangerous?

Maybe I miss something but I don't see any ACL checks at all here.
The save method uses $this->allowSave($data, $key) to check if saving is allowed which I don't see in your reload method.
We may also have to use checkEditId($context, $id) to verify that the item was correctly opened before.

Why is the allowSave check needed, it doesn't save, it just reloads the page to an url like /cms/administrator/index.php?option=com_content&view=article&layout=edit&id=1. The checkedit id is done in the controller itself. I mean at the end it performs the same action as when you want to edit the article. If you paste that url directly into the browser and you didn't go trough the edit task, then it kicks you back to the list with a warning to not get directly to the edit page. I'v just tested it and it worked as expected.
If I'm now not completely wrong then I don't understand your points here. What else needs to be checked?

Why is the allowSave check needed, it doesn't save, it just reloads the page to an url like /cms/administrator/index.php?option=com_content&view=article&layout=edit&id=1.

It does save the data into the session. It at least feels wrong to have a controller task without any ACL check. Which means anyone could run it. It may not be an issue by itself, but it may bite us back and there is no reason this task should be available unauthorised. Using allowSave or allowEdit (if that fits better due to not saving into db) is a save check we can and should do imho. I don't see a reason to not do it.

As for checkEditId, you're right. I just remembered we check it but didn't remember where exactly ?

Does this not allow accessing data from any JControllerForm instance (including in the frontend (e.g. com_contact)) without checking the published/acl levels for the item though?

I don't se how, it only stores the jform data into the session.

It is still a POST request receiving user data. It should therefore be properly validated and access controlled. Especially if that data stored into the session can be used in a follow on request (which HAS been used in security issues we have fixed in the last 2 years).

The session is data that the user controls. That means the user has the data which they shouldn't have been allowed to access (for ACL reasons) - and even if it isn't displayed on the page. It's trivial to get the session data if you wanted it.

Does that help d10e8fc?

Imho, the ACL check is now fine.

Looking at the redirects, why don't you just use $this->redirect()? Also $app->close() is useless even when you use $app->redirect() directly, since that method already closes the app. Or do I miss something?

Just to be sure. Probably not needed.

If there is no specific reason to use $app->redirect() directly, can you just use $this->setRedirect($url, $msg, $type) and $this->redirect() then? So we are consistent with how we process redirects in other places in the controllers.

True, done.

RE ACL: I'm honestly unsure given what this function does whether it should be edit or read acl permissions we should be using. But this looks significantly better now :)

It should check edit, as it belongs to the edit action.

@laoneo
I can't get the calendar field anymore
( ! ) Warning: Invalid argument supplied for foreach() in /Applications/MAMP/htdocs/testnew/trunkgitnew/libraries/cms/helper/content.php on line 74

Good catch. Fixed it in the last commits.

Works now. Remains the necessary changes in script.php

Ok, done.

I have tested this item ✅ successfully on fb925e7

I have tested this item ✅ successfully on fb925e7

RTC after two successful tests.

Thanks guys!