This repository has been archived by the owner. It is now read-only.

JSession still uses JRequest #1180

Merged
merged 4 commits into from May 10, 2012

Conversation

Projects
None yet
3 participants
@florianv
Contributor

florianv commented May 2, 2012

It causes a fatal error when instanciating JSession because _start() still uses JRequest. I don't know if you want to use this construction

@realityking

View changes

libraries/joomla/session/session.php
{
- if (JRequest::getVar($session_name))
+ if ($session_clean = $input->get($session_name, false, 'string'))

This comment has been minimized.

@realityking

realityking May 3, 2012

Contributor

Please don't make assignments inside conditionals. We want to move away from that kind of style since it's a source of bugs and confusion.

@realityking

realityking May 3, 2012

Contributor

Please don't make assignments inside conditionals. We want to move away from that kind of style since it's a source of bugs and confusion.

@realityking

This comment has been minimized.

Show comment
Hide comment
@realityking

realityking May 3, 2012

Contributor

Otherwise looks good, will test with the CMS after the adjustment is made.

Contributor

realityking commented May 3, 2012

Otherwise looks good, will test with the CMS after the adjustment is made.

@florianv

This comment has been minimized.

Show comment
Hide comment
@florianv

florianv May 3, 2012

Contributor

Thanks for the feedback. I didn't find an efficient way to test it

Contributor

florianv commented May 3, 2012

Thanks for the feedback. I didn't find an efficient way to test it

@realityking

This comment has been minimized.

Show comment
Hide comment
@realityking

realityking May 3, 2012

Contributor

Thanks for the fix. You now have a code style error on line 534 (whitespace at the end of a line)

Contributor

realityking commented May 3, 2012

Thanks for the fix. You now have a code style error on line 534 (whitespace at the end of a line)

@florianv

This comment has been minimized.

Show comment
Hide comment
@florianv

florianv May 3, 2012

Contributor

I think Eclipse adds it, found a plugin that should remove it, I hope it works

Contributor

florianv commented May 3, 2012

I think Eclipse adds it, found a plugin that should remove it, I hope it works

chdemko added a commit that referenced this pull request May 10, 2012

Merge pull request #1180 from florianv/fix1
JSession still uses JRequest

@chdemko chdemko merged commit 0df15e3 into joomla:staging May 10, 2012

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.