I have tested this item ✅ successfully on

Looks like working here.

i have experienced this issue on GsoC 2016 Multilingual project.

test conditions

• Create a user "test" added to "Administrator" group

• Go to com_content Options, Permissions tab and disable "Edit" for "Administrator" group
  

• Create a new content item with the "test" user

before patch
You can't edit your own.

Apply patch
you can edit your own items.

p.s
tested on backend only

I have tested this item ✅ successfully on

I have tested this item ✅ successfully on
Thanks a lot!

RTC. Thanks

I have tested this item successfully. Thank's.

Put off RTC so we get a retest after the last changes Thanks.

yes i made 1 more change (besides comments),

• always return false if article id is 0

I searched through all calls of the method, i did not find any usage that will need to return component permissions, in such a case,

so better return false (aka deny), in case someone in the future writes code to misuse this

Except this is wrong. You've introduced a hard deny rule instead of continued use of the ACL system in this scenario. So there needs to be a pretty freakin' strong validation against this change because right now that screams massive B/C break to me.

Can you find a place that is needed today ?
And something that gets broken ?

example

    protected function allowSave($data, $key = 'id')
    {
        $recordId = isset($data[$key]) ? $data[$key] : '0';

        if ($recordId)
        {
            return $this->allowEdit($data, $key);
        }
        else
        {
            return $this->allowAdd($data);
        }
    }

so calling allowEdit() on zero record id as an indirect way to get return parent controler permissions ?
which return the component permission,

is weird usage

i will revert, i made this change worrying of someone misusing it in the future
it creates no problems

I don't have a place off hand that will break with that hardcoded change but the point of it is that something that should be reading the ACL system should be doing that, not introducing an arbitrary rule based on some condition and not even implement it consistently.

I restored the code it is same as before, when all testing was done

but that deny would break nothing anyway, because it is not used anywhere

anyway this is PR makes the allowEdit how it should have been in the first place

I agree it shouldn't be getting used, but in the off case someone's code is screwed up somewhere it should still respect the ACL system or throw an Exception because at that point it's dealing with an invalid record anyway.

The code is correct to be there
i am not saying something different

but after 1 or 2 years someone may write code to misuse it

• that was my only reason for making that change

It needs to be a separate proposal if you're going to push for it. We all have a very bad habit of throwing unrelated items into pull requests because we're already making a change or want to enforce some arbitrary standard. That crap needs to stop.

It needs to be a separate proposal if you're going to push for it. We all have a very bad habit of throwing unrelated items into pull requests because we're already making a change or want to enforce some arbitrary standard. That crap needs to stop.

yes correct, it is unrelated to this PR, but you know how it is with finding people to test PRs etc

Oh I know, I know. I've got cache PRs stalled out because someone asked for an arbitrary extra change, one of which actually adds support for a PHP 5.3 feature to our callback system.

where do we stand here?

Hello

please click on each of the commit made August 10,
https://github.com/joomla/joomla-cms/pull/11511/commits

5 commits

• 1 commit changes comments
• 2 next commits changed in the 2 controllers (frontend / backend identical), for the case of record ID zero

        if (!$recordId)
        {
-           return parent::allowEdit($data, $key);
+           return false;
        }

this should be still be RTC

• 2 more commits changed it back to be the same as when all tests where made

        if (!$recordId)
        {
-           return false;
+           return parent::allowEdit($data, $key);
        }

Back to RTC. Thanks

I have tested this item ✅ successfully on 84d0dad