don't know exactly why this behaviour either. can't advise.

I think it would be good if we could implement this redirection mechanism in the list views too. This will also get rid of the annoying resubmit form confirmation from browser on refresh of the list views.

Sound good, improvment but not a solution to "fix" the real issue :)

@Devportobello The fix for the real issue is as simple as adding a $this->getState(); call at the top of loadFormData method body.

i.e. Just before the line 446 in the above screenshot

This will work but it is maybe like a hack,

• i may be wrong but please readon there is more proper fix,

https://github.com/joomla/joomla-cms/blob/staging/libraries/legacy/model/legacy.php#L434-L446
as mentioned above the
getState() of class JModelList (inherited by JModelLegacy)
makes sure that ... populateState() has been called

• and if a property is requested it gives by returning:

$this->state->get($property, $default)

now please look at code of loadFormData() from class JModelList

https://github.com/joomla/joomla-cms/blob/staging/libraries/legacy/model/list.php#L444-L461

protected function loadFormData()
{
    // Check the session for previously entered form data.
    $data = JFactory::getApplication()->getUserState($this->context, new stdClass);

    // Pre-fill the list options
    if (!property_exists($data, 'list'))
    {
        $data->list = array(
            'direction' => $this->state->{'list.direction'},
            'limit'     => $this->state->{'list.limit'},
            'ordering'  => $this->state->{'list.ordering'},
            'start'     => $this->state->{'list.start'}
        );
    }

    return $data;
}

Why are we using this inside the model: $this->state->{'list.direction'}

instead of using: $this->getState('list.direction')

• the above is ok to use inside the layout, but no inside the model, because it defeats the purpose of $this->getState(), which purpose is to populate the state on 1st call
• almost all other code of this class (JModelList) and all code of extended model classes does it the proper way and uses: $this->getState()

so the issue is that this code needs to be updated and use getState(),

maybe i am wrong

But i see there is already a PR for this ?, i missed it

ok, @izharaazmi i understand now

the assumption that populateState() needs to be called,
only when reading a property from state is wrong

• i see (remember) now that populateState() also does has calls to:

setUserState()

so indeed this is a different but relevant issue to what this PR has patched:

Use getState instead of direct property read
#10435

in our custom component we call populateState() or a relevant function in the constructor, it is sometimes wasted, but avoids these problems

Calling populateState() in the constructor is a bad practice/idea, IMO. Rather we can always call getState method wherever we feel that the subsequent code is expected to depend on the model state. e.g. - in the view classes. See! getState is a public function :)

Well maybe it is but practice, but

• constructors are supposed to do initialization,
• about what is bad practice please read close to the bottom of my answer

Now some times populateState() in the constructor can be wasted

• but if some specific classes only get very few instatiations, then this makes almost zero difference in terms of performance (e.g. item listing classes)
• my concern about calling populateState() or similar function inside the constructor, is only if it will introduce any bugs in my class

I do it most of my JModelList extending classes (not all), then in rare case that at a later time we discover some change that needs to go into state, i update the state

don't get me wrong, i just do what my extension needs to work properly in all cases,

• i don't say to do it here

Now the getUserState() assumes, that some good felow has already called getState()

And the above, is bad practice, becauses it hopes that something has been initialized but does not really check that it was

e.g. if this is fixed inside loadFormData, then

1. there is no warning to people extending the class, that they also need to update their loadFormData() for their code to avoid the same bug, because we have not fixed the real issue with getUserState
2. Furthermore other class methods may have similar issue in the future

maybe fix should inside getUserState()
( and to play safe maybe only inside the JModelList and not to parent class ?)

1. check if class has method populateState()
2. call it using the SAME IF statement used by getState()

i don't know if the above will create any other issues, (*but i don't think it will create any, why would it ? *) i just say it sounds more proper, that just patching loadFormData()

@izharaazmi
don't get me wrong thanks for all your effort and contributions, you are spending time fixing it

• and then time trying to get your fix merged

i have spent some considerable time to study this, but surely less than you

@ggppdk ???? I'm not getting you wrong! We are discussing for improvement. Why would I?

there is no warning to people extending the class, that ....

Agree. But do we need to? People extending and overriding the method are responsible for their implementation logic and they are likely aware with this. And we're not breaking compatibility, right?

maybe fix should inside getUserState()....

No, getUserState is a global level method and it doesn't have to be aware of which model(s) are in context and what data they hold. Its the responsibility of the models or other contexts to update it whenever needed.

IMO, calling getState inside loadFormData is not enough. This issue may come up with other methods some day. And that is why I didn't PR with this simple fix.

Better is if the developers understand that whenever they call something that depends on the (updated) model state or application state, they need to call getState (once) first. I know this is not easy to convince.

So why don't we implement the redirection mechanism as in case of Form views where the same works flawlessly because the state is anyway populated in the previous request already.

To be honest, I saw this behaviour when I used to work with J1.5.x. I never called it an issue and taught myself to always call $this->get('state'); before I call getItems or getPagination etc. on the model.

So as of my understanding the summary of this is either of the two below (quoting from #10448 (comment) above):

• Don't change anything in Joomla and let the developers understand that they need to call getState() from wherever they want an updated state data.

• Change the Joomla behaviour to always use redirection when the form is submitted and hence convert it to GET request. So that the state is always up-to-date.

In first case this issue can be safely closed. In that later case, suggest me and I'd try to submit a PR.

@rdeutz @wilsonge can you advise please on https://issues.joomla.org/tracker/joomla-cms/10448#event-173724 so that @izharaazmi knows if this should be closed or if he should create a PR

My personal opinion is that I don't like the redirect concept, I think it has created more problems as it solved. I also think developers should know what they are doing and understanding the request cycle. If you need a specific situation in your code then make sure anything you are going to use is properly set. I would say we can close this here.

Thanks @rdeutz and everyone for contributing
Closing