[#9276] - set() returns the old value of the variable
- Closed
- 2 Mar 2016
- Medium
- Build: staging
- # 9276
- Diff
- GABBAR1947:test
- Pending Human Test Results: 1 Successful 0 Failed. Details
User tests: Successful: Unsuccessful:
Pull Request for Issue #8859
Testing instructions remain the same as referenced in the issue:
$session = JFactory::getSession();
$session->set('test', 1);
echo $session->set('test', 2);
| Status | New | ⇒ | Pending |
joomla-cms-bot
- | Labels |
Added:
?
|
||
| Labels |
Added:
?
|
||
I have tested this item 
successfully on 114ccb1
Tested with success with code snippet provided here and in #8859 and with more detailed test instructions from #8859 .
Works as described.
The code change makes sense when reading, too.
But since I am not expert in Joomla! session handling I cannot judge if the documentation and this PR are correct (very likely), or if maybe the change in how the function works was by purpose (very unlikely, just wanted to mention that).
This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/9276.
It wasn't on purpose - I can confirm that!
Then this PR here seems to solve it in the right way, yes?
This is definitely the right thing to do - just needs 1 more test :P
@wilsonge What is strange is that on GitHub in this PR, there are 2 bad commits always shown at the end as done just now here in the conversation tab and also in the commits tab, but when looking at the changed files in this PR they seem to be outdated, and the final result of this PR looks good regarding code style, what these 2 commits don't. So there seems to be something wrong with display here (maybe a NULL date in the point of time of these 2 commits in GitHub?). On the issue tracker the history looks OK.
Looks like a GitHub problem, not related to this PR. Just wanted to mention.
@GABBAR1947 It seems like you've messed up the commits. Could you please create a new, clean PR with only the session change? Once done, please reference on this PR and close it. Thanks!
| Status | Pending | ⇒ | Closed |
| Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2016-03-02 14:12:37 |
| Closed_By | ⇒ | GABBAR1947 |
@Kubik-Rubik Hmm, I hope he not missunderstood and closed without redo.
This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/9276.
@richard67 No, I'm coming up with a new one. Actually the moment I commit the code, there appears to be an indentation issue.
Yah I noticed that in some other github PR's as well - as you say likely a github issue - I guess we just have to wait for them to clear it up :/
| Labels |
Removed:
?
|
||
Adding 3.5 blocking tag on this as it's a b/c break introduced in 3.4.8