Add a plugin with a nag message to update PHP #11498

Closed
wants to merge 12 commits into
from

Conversation

Projects
None yet
@mbabker
Member

mbabker commented Aug 6, 2016

Summary of Changes

Far too many people run Joomla on outdated PHP versions (per the stats only 37% of Joomla! 3.5+ installs are on a currently supported PHP version). To help alleviate this, I propose a nag message to tell people to upgrade.

This plugin checks three conditions: whether the PHP version is still fully supported, whether it has entered security support only, or whether it is unsupported by the PHP project. I don't particularly care whether the user is running a PHP version custom compiled by some Linux distro that claims they are still supporting it; if PHP isn't supporting the branch anymore they need to update.

Testing Instructions

Apply the pull request; discover, install, and enable the plugin. For PHP 5.3 thru 5.5 you should get a red error message telling you the PHP version is unsupported and you should upgrade yesterday. PHP 5.6 and 7.0 won't emit anything because neither has reached a security only state, however if you want to test that aspect of things go into the plugin and change the date for when security support ends in the data array to a date before today (today being whatever day you test this).

Documentation Changes Required

None. No new API is introduced.

Maintenance Efforts Added

As the support dates for PHP branches change (new versions added, dates postponed), the data array in this plugin will need to be updated. Generally this is a once a year requirement when a new version is added only.

Joomla, Stop Nagging Me!!!

Upgrade, plain and simple. Oh, not an option? As long as you understand you are running unsupported software, you can disable the plugin and stop being nagged. But you should still upgrade yesterday.

+; Note : All ini files need to be saved as UTF-8
+
+PLG_SYSTEM_PHPVERSIONCHECK="System - PHP Version Check"
+PLG_SYSTEM_PHPVERSIONCHECK_SECURITY_ONLY="Your PHP version, %1$s, is only receiving security fixes at this time. This means your PHP version will soon no longer be supported. We recommend planning to upgrade to a newer PHP version before it reaches end of support on %2$s. Please contact your host for upgrade instructions."

This comment has been minimized.

@brianteeman

brianteeman Aug 6, 2016

Contributor

i am concerned that non technical users dont know what php is. in the other message you say "by the PHP project."
Can you change this message to
"s only receiving security fixes at this time from the PHP project."

@brianteeman

brianteeman Aug 6, 2016

Contributor

i am concerned that non technical users dont know what php is. in the other message you say "by the PHP project."
Can you change this message to
"s only receiving security fixes at this time from the PHP project."

@brianteeman

This comment has been minimized.

Show comment
Hide comment
@brianteeman

brianteeman Aug 6, 2016

Contributor

i am concerned that non technical users dont know what php is.

Contributor

brianteeman commented Aug 6, 2016

i am concerned that non technical users dont know what php is.

@brianteeman

This comment has been minimized.

Show comment
Hide comment
@brianteeman

brianteeman Aug 6, 2016

Contributor

It would be beneficial if this explained the benefits of upgrading the php version and how easy it usually is

Contributor

brianteeman commented Aug 6, 2016

It would be beneficial if this explained the benefits of upgrading the php version and how easy it usually is

@mbabker

This comment has been minimized.

Show comment
Hide comment
@mbabker

mbabker Aug 6, 2016

Member

i am concerned that non technical users dont know what php is.

Well without turning this into a full screen notification, hard to really explain it. Totally valid concern, but not easy to deal with. Unlike other projects who encourage users to not need to be technical, I think it's borderline irresponsible to not at least know what platform(s) your site is running on.

Member

mbabker commented Aug 6, 2016

i am concerned that non technical users dont know what php is.

Well without turning this into a full screen notification, hard to really explain it. Totally valid concern, but not easy to deal with. Unlike other projects who encourage users to not need to be technical, I think it's borderline irresponsible to not at least know what platform(s) your site is running on.

@brianteeman

This comment has been minimized.

Show comment
Hide comment
@brianteeman

brianteeman Aug 6, 2016

Contributor

I live in the real world not the technical world. ;)

If you dont explain the benefits or how easy it usually is to upgrade we will lose users who just assume their hosting doesnt support joomla.

I am not against this concept just the implementation

Contributor

brianteeman commented Aug 6, 2016

I live in the real world not the technical world. ;)

If you dont explain the benefits or how easy it usually is to upgrade we will lose users who just assume their hosting doesnt support joomla.

I am not against this concept just the implementation

@mbabker

This comment has been minimized.

Show comment
Hide comment
@mbabker

mbabker Aug 6, 2016

Member

Suggestions welcome. I'm just trying to keep this from becoming a full screen notification.

Member

mbabker commented Aug 6, 2016

Suggestions welcome. I'm just trying to keep this from becoming a full screen notification.

@brianteeman

This comment has been minimized.

Show comment
Hide comment
@brianteeman

brianteeman Aug 6, 2016

Contributor

We highly recommend upgrading your server to a newer PHP version. Please contact your host for upgrade instructions."

==>

Joomla will be faster and more secure if you upgrade to a newer PHP version (preferably 7.x). Contact your host - they can usually do this for you very easily.

Contributor

brianteeman commented Aug 6, 2016

We highly recommend upgrading your server to a newer PHP version. Please contact your host for upgrade instructions."

==>

Joomla will be faster and more secure if you upgrade to a newer PHP version (preferably 7.x). Contact your host - they can usually do this for you very easily.

@brianteeman

This comment has been minimized.

Show comment
Hide comment
@brianteeman

brianteeman Aug 6, 2016

Contributor

For reference our previous message was

COM_CPANEL_MSG_PHPVERSION_BODY="Beginning with Joomla! 3.3, the version of PHP this site is using will no longer be supported. Joomla! 3.3 will require at least <a href="QQ"http://community.joomla.org/blogs/leadership/1798-raising-the-bar-on-security.html"_QQ_">PHP version 5.3.10 in order to provide enhanced security features to its users."

Contributor

brianteeman commented Aug 6, 2016

For reference our previous message was

COM_CPANEL_MSG_PHPVERSION_BODY="Beginning with Joomla! 3.3, the version of PHP this site is using will no longer be supported. Joomla! 3.3 will require at least <a href="QQ"http://community.joomla.org/blogs/leadership/1798-raising-the-bar-on-security.html"_QQ_">PHP version 5.3.10 in order to provide enhanced security features to its users."

+; Note : All ini files need to be saved as UTF-8
+
+PLG_SYSTEM_PHPVERSIONCHECK="System - PHP Version Check"
+PLG_SYSTEM_PHPVERSIONCHECK_SECURITY_ONLY="Your PHP version, %1$s, is only receiving security fixes at this time from the PHP project. This means your PHP version will soon no longer be supported. We recommend planning to upgrade to a newer PHP version before it reaches end of support on %2$s. Please contact your host for upgrade instructions."

This comment has been minimized.

@brianteeman

brianteeman Aug 6, 2016

Contributor

can you add this sentence to this string as well

Joomla will be faster and more secure if you upgrade to a newer PHP version (PHP 7.x is recommended).

@brianteeman

brianteeman Aug 6, 2016

Contributor

can you add this sentence to this string as well

Joomla will be faster and more secure if you upgrade to a newer PHP version (PHP 7.x is recommended).

@zero-24

This comment has been minimized.

Show comment
Hide comment
@zero-24

zero-24 Aug 6, 2016

Contributor

The new language files needs to be added to the install.xml ;)

Contributor

zero-24 commented Aug 6, 2016

The new language files needs to be added to the install.xml ;)

@zero-24

This comment has been minimized.

Show comment
Hide comment
@zero-24

zero-24 Aug 6, 2016

Contributor

And the update sql ;P

Contributor

zero-24 commented Aug 6, 2016

And the update sql ;P

@mbabker

This comment has been minimized.

Show comment
Hide comment
@mbabker

mbabker Aug 6, 2016

Member

And the update sql ;P

What update SQL?

Member

mbabker commented Aug 6, 2016

And the update sql ;P

What update SQL?

@zero-24

This comment has been minimized.

Show comment
Hide comment
@zero-24

zero-24 Aug 6, 2016

Contributor

That it get enabled on update. The update sql in com_admin :)

Contributor

zero-24 commented Aug 6, 2016

That it get enabled on update. The update sql in com_admin :)

@zero-24

This comment has been minimized.

Show comment
Hide comment
@zero-24

zero-24 Aug 6, 2016

Contributor

Ignore that i have just not saw it. Sorry

Contributor

zero-24 commented Aug 6, 2016

Ignore that i have just not saw it. Sorry

@roland-d

This comment has been minimized.

Show comment
Hide comment
@roland-d

roland-d Aug 6, 2016

Contributor

Should this message be shown to super users/administrators only perhaps? We may assume they know what PHP is.

Contributor

roland-d commented Aug 6, 2016

Should this message be shown to super users/administrators only perhaps? We may assume they know what PHP is.

@mbabker

This comment has been minimized.

Show comment
Hide comment
@mbabker

mbabker Aug 6, 2016

Member

Should this message be shown to super users/administrators only perhaps? We may assume they know what PHP is.

Even that's an overly broad assumption.

Member

mbabker commented Aug 6, 2016

Should this message be shown to super users/administrators only perhaps? We may assume they know what PHP is.

Even that's an overly broad assumption.

@brianteeman

This comment has been minimized.

Show comment
Hide comment
@brianteeman

brianteeman Aug 6, 2016

Contributor
Contributor

brianteeman commented Aug 6, 2016

@roland-d

This comment has been minimized.

Show comment
Hide comment
@roland-d

roland-d Aug 6, 2016

Contributor

In that case just show it to everyone and they can google what PHP is.

Contributor

roland-d commented Aug 6, 2016

In that case just show it to everyone and they can google what PHP is.

@brianteeman

This comment has been minimized.

Show comment
Hide comment
@brianteeman

brianteeman Aug 6, 2016

Contributor
Contributor

brianteeman commented Aug 6, 2016

@mbabker

This comment has been minimized.

Show comment
Hide comment
@mbabker

mbabker Aug 6, 2016

Member

Right now it shows on the backend to everyone similarly to the 2.5 EOS plugin.

Member

mbabker commented Aug 6, 2016

Right now it shows on the backend to everyone similarly to the 2.5 EOS plugin.

@andrepereiradasilva

This comment has been minimized.

Show comment
Hide comment
@andrepereiradasilva

andrepereiradasilva Aug 6, 2016

Contributor

i don't have any server to test this since my servers php version are all 7.x ... ok. that it's probably good news 😄

Contributor

andrepereiradasilva commented Aug 6, 2016

i don't have any server to test this since my servers php version are all 7.x ... ok. that it's probably good news 😄

@infograf768

This comment has been minimized.

Show comment
Hide comment
@infograf768

infograf768 Aug 7, 2016

Member

Should be tagged to New Feature I guess.


This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/11498.

Member

infograf768 commented Aug 7, 2016

Should be tagged to New Feature I guess.


This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/11498.

@alikon

This comment has been minimized.

Show comment
Hide comment
@alikon

alikon Aug 7, 2016

Contributor

its a little bit too much nag in this way
toomuch

Contributor

alikon commented Aug 7, 2016

its a little bit too much nag in this way
toomuch

@AlexRed

This comment has been minimized.

Show comment
Hide comment
@AlexRed

AlexRed Aug 7, 2016

Contributor

I have tested this item successfully on 866376b


This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/11498.

Contributor

AlexRed commented Aug 7, 2016

I have tested this item successfully on 866376b


This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/11498.

@PhilETaylor

This comment has been minimized.

Show comment
Hide comment
@PhilETaylor

PhilETaylor Aug 7, 2016

Contributor

I recommend not using YYYY-MM-DD as internationally this can be read as YYYY-DD-MM by some... as proved on 2016-08-03 when I got 7 birthday wishes from services :)

Contributor

PhilETaylor commented Aug 7, 2016

I recommend not using YYYY-MM-DD as internationally this can be read as YYYY-DD-MM by some... as proved on 2016-08-03 when I got 7 birthday wishes from services :)

@rdeutz

This comment has been minimized.

Show comment
Hide comment
@rdeutz

rdeutz Aug 7, 2016

Contributor

We should add a link to a page on docs where we explain it with more details, can be also translated into other languages.

Contributor

rdeutz commented Aug 7, 2016

We should add a link to a page on docs where we explain it with more details, can be also translated into other languages.

@andrepereiradasilva

This comment has been minimized.

Show comment
Hide comment
Contributor

andrepereiradasilva commented Aug 7, 2016

i would just make a link to https://secure.php.net/supported-versions.php

@rdeutz

This comment has been minimized.

Show comment
Hide comment
@rdeutz

rdeutz Aug 7, 2016

Contributor

@andrepereiradasilva that's too technical, I would prefer something that explains the reason and how to do it on a non technical level

Contributor

rdeutz commented Aug 7, 2016

@andrepereiradasilva that's too technical, I would prefer something that explains the reason and how to do it on a non technical level

@ggppdk

This comment has been minimized.

Show comment
Hide comment
@ggppdk

ggppdk Aug 7, 2016

Contributor

Some notes:

1 . I have similar nagging warnings about PHP 5.3 and about PHP configuration,

so i want add my opinion here

  • what about adding a counter to session after it appears 3 times for current session then go away

why the above is good to do ?
because if it appears too many times, then users will disable the plugin and forget about it,
but if it appear e.g. 3-4 times then people may let the plugin enabled and eventually more people will upgrade

2 .

i would just make a link to https://secure.php.net/supported-versions.php

excellent idea if all users were advanced users, I think current messages are a little more work for maintaining the plugin, but faster / easier to read for most users

  • still i think adding the link to the page with "read more here" is ok as long as we do not remove the inline messages

3 . The message appearing in backend login screen is an oversight right ?, we do not want it there

Contributor

ggppdk commented Aug 7, 2016

Some notes:

1 . I have similar nagging warnings about PHP 5.3 and about PHP configuration,

so i want add my opinion here

  • what about adding a counter to session after it appears 3 times for current session then go away

why the above is good to do ?
because if it appears too many times, then users will disable the plugin and forget about it,
but if it appear e.g. 3-4 times then people may let the plugin enabled and eventually more people will upgrade

2 .

i would just make a link to https://secure.php.net/supported-versions.php

excellent idea if all users were advanced users, I think current messages are a little more work for maintaining the plugin, but faster / easier to read for most users

  • still i think adding the link to the page with "read more here" is ok as long as we do not remove the inline messages

3 . The message appearing in backend login screen is an oversight right ?, we do not want it there

@brianteeman

This comment has been minimized.

Show comment
Hide comment
@brianteeman

brianteeman Aug 7, 2016

Contributor

Not in favour of this message being displayed to unauthenticated users

Contributor

brianteeman commented Aug 7, 2016

Not in favour of this message being displayed to unauthenticated users

@andrepereiradasilva

This comment has been minimized.

Show comment
Hide comment
@andrepereiradasilva

andrepereiradasilva Aug 7, 2016

Contributor

Not in favour of this message being displayed to unauthenticated users

Agree

Contributor

andrepereiradasilva commented Aug 7, 2016

Not in favour of this message being displayed to unauthenticated users

Agree

@rdeutz

This comment has been minimized.

Show comment
Hide comment
@rdeutz

rdeutz Aug 7, 2016

Contributor

agreed, never ever on the login screen

Contributor

rdeutz commented Aug 7, 2016

agreed, never ever on the login screen

@rdeutz

This comment has been minimized.

Show comment
Hide comment
@rdeutz

rdeutz Aug 7, 2016

Contributor

I have tested this item successfully on 866376b

works as expected, also changed the security date for 5.6 and the message popped up


This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/11498.

Contributor

rdeutz commented Aug 7, 2016

I have tested this item successfully on 866376b

works as expected, also changed the security date for 5.6 and the message popped up


This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/11498.

@mbabker

This comment has been minimized.

Show comment
Hide comment
@mbabker

mbabker Aug 7, 2016

Member

Trying to go through this all in one comment:

I recommend not using YYYY-MM-DD as internationally this can be read as YYYY-DD-MM by some... as proved on 2016-08-03 when I got 7 birthday wishes from services :)

@PhilETaylor It uses the DATE_FORMAT_LC4 key so it gets localized to the language's definition. For the Americans using British English if you still haven't figured out that Joomla uses MM-DD instead of DD-MM you've got other issues 😉

Not in favour of this message being displayed to unauthenticated users

@brianteeman Patched it.

i also think this is warning not an error.
So please change the message type to warning.

@andrepereiradasilva I disagree. It is a "major" warning that the user is running unsupported software. We should not be "polite" about the type of warning we raise in this case. It's the same reaction as when users come up reporting issues on Joomla 1.5 or 2.5 installs, usually the first response is "upgrade".

The intent is not to give the user a kind feeling here. They need to be fully aware that they are using software that no longer receives support. If we want a "hold the user's hand and make them feel good" approach, then shove this in the postinstall messages screen so it can be ignored. If we want to do something proactive, this is the way to go. Granted the language and references can be massively improved, but I don't want this to turn into something the user's going to blow off just because we made the message far too friendly.

Member

mbabker commented Aug 7, 2016

Trying to go through this all in one comment:

I recommend not using YYYY-MM-DD as internationally this can be read as YYYY-DD-MM by some... as proved on 2016-08-03 when I got 7 birthday wishes from services :)

@PhilETaylor It uses the DATE_FORMAT_LC4 key so it gets localized to the language's definition. For the Americans using British English if you still haven't figured out that Joomla uses MM-DD instead of DD-MM you've got other issues 😉

Not in favour of this message being displayed to unauthenticated users

@brianteeman Patched it.

i also think this is warning not an error.
So please change the message type to warning.

@andrepereiradasilva I disagree. It is a "major" warning that the user is running unsupported software. We should not be "polite" about the type of warning we raise in this case. It's the same reaction as when users come up reporting issues on Joomla 1.5 or 2.5 installs, usually the first response is "upgrade".

The intent is not to give the user a kind feeling here. They need to be fully aware that they are using software that no longer receives support. If we want a "hold the user's hand and make them feel good" approach, then shove this in the postinstall messages screen so it can be ignored. If we want to do something proactive, this is the way to go. Granted the language and references can be massively improved, but I don't want this to turn into something the user's going to blow off just because we made the message far too friendly.

@conconnl

This comment has been minimized.

Show comment
Hide comment
@conconnl

conconnl Aug 7, 2016

This is indeed a great way to push people to higher PHP versions.

I would make it an error and not a warning, an warning is too soft, especially when you want to remove to support for certain PHP versions in the next major.

Adding a link to a Joomla doc with information for the normal users and system administrators is an necessary thing, too make sure the error does not get to big. Helping with creating this doc is no problem.

Never show information like this on the login screen, if we do this we would great a possible security problem. Because we would give outsiders the information that the host is not up to date on PHP level.

I will test this PR tomorrow on the different PHP versions.

conconnl commented Aug 7, 2016

This is indeed a great way to push people to higher PHP versions.

I would make it an error and not a warning, an warning is too soft, especially when you want to remove to support for certain PHP versions in the next major.

Adding a link to a Joomla doc with information for the normal users and system administrators is an necessary thing, too make sure the error does not get to big. Helping with creating this doc is no problem.

Never show information like this on the login screen, if we do this we would great a possible security problem. Because we would give outsiders the information that the host is not up to date on PHP level.

I will test this PR tomorrow on the different PHP versions.

@mbabker

This comment has been minimized.

Show comment
Hide comment
@mbabker

mbabker Aug 7, 2016

Member

This is where we're at now for unsupported. And yes I did fudge the code to fake the check because the only PHP versions I have functional on my laptop these days are 5.6 and 7.0.

screen shot 2016-08-07 at 11 28 57 am

Member

mbabker commented Aug 7, 2016

This is where we're at now for unsupported. And yes I did fudge the code to fake the check because the only PHP versions I have functional on my laptop these days are 5.6 and 7.0.

screen shot 2016-08-07 at 11 28 57 am

@brianteeman

This comment has been minimized.

Show comment
Hide comment
@brianteeman

brianteeman Aug 7, 2016

Contributor

it is not an error by any definition that we use the term error currently

i we start to use error when it is not an error then people will ignore
real errors. its "sheep in wolves clothing syndrome"

On 7 August 2016 at 17:29, Michael Babker notifications@github.com wrote:

This is where we're at now for unsupported. And yes I did fudge the code
to fake the check because the only PHP versions I have functional on my
laptop these days are 5.6 and 7.0.

[image: screen shot 2016-08-07 at 11 28 57 am]
https://cloud.githubusercontent.com/assets/368545/17463731/3423a7f2-5c92-11e6-8ee8-d6e5b25a7d14.png


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#11498 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/ABPH8Zhlmof68zoXWAuqtSPdoNvD6PJbks5qdggFgaJpZM4JeXQf
.

Brian Teeman
Co-founder Joomla! and OpenSourceMatters Inc.
http://brian.teeman.net/

Contributor

brianteeman commented Aug 7, 2016

it is not an error by any definition that we use the term error currently

i we start to use error when it is not an error then people will ignore
real errors. its "sheep in wolves clothing syndrome"

On 7 August 2016 at 17:29, Michael Babker notifications@github.com wrote:

This is where we're at now for unsupported. And yes I did fudge the code
to fake the check because the only PHP versions I have functional on my
laptop these days are 5.6 and 7.0.

[image: screen shot 2016-08-07 at 11 28 57 am]
https://cloud.githubusercontent.com/assets/368545/17463731/3423a7f2-5c92-11e6-8ee8-d6e5b25a7d14.png


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#11498 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/ABPH8Zhlmof68zoXWAuqtSPdoNvD6PJbks5qdggFgaJpZM4JeXQf
.

Brian Teeman
Co-founder Joomla! and OpenSourceMatters Inc.
http://brian.teeman.net/

@mbabker

This comment has been minimized.

Show comment
Hide comment
@mbabker

mbabker Aug 7, 2016

Member

Well we've already done that, just look at the "Joomla! 3.6.2 is available" notification right above.

Member

mbabker commented Aug 7, 2016

Well we've already done that, just look at the "Joomla! 3.6.2 is available" notification right above.

@andrepereiradasilva

This comment has been minimized.

Show comment
Hide comment
@andrepereiradasilva

andrepereiradasilva Aug 7, 2016

Contributor

i agree with brian that should be a warning.

Well we've already done that, just look at the "Joomla! 3.6.2 is available" notification right above.

repeating a problem, doesn't make it right...

Contributor

andrepereiradasilva commented Aug 7, 2016

i agree with brian that should be a warning.

Well we've already done that, just look at the "Joomla! 3.6.2 is available" notification right above.

repeating a problem, doesn't make it right...

@mbabker

This comment has been minimized.

Show comment
Hide comment
@mbabker

mbabker Aug 7, 2016

Member

So by the argument we have on that page right now, contextually it's an error if a user doesn't update Joomla or its extensions but it's only a warning if a user doesn't update PHP?

Member

mbabker commented Aug 7, 2016

So by the argument we have on that page right now, contextually it's an error if a user doesn't update Joomla or its extensions but it's only a warning if a user doesn't update PHP?

@ggppdk

This comment has been minimized.

Show comment
Hide comment
@ggppdk

ggppdk Aug 7, 2016

Contributor

i we start to use error when it is not an error then people will ignore

Keep it a error message (red)

  • but show it only at control panel
  • if also showing it at other pages, then display it no more than a max of 3 times per session in non-control panel pages
Contributor

ggppdk commented Aug 7, 2016

i we start to use error when it is not an error then people will ignore

Keep it a error message (red)

  • but show it only at control panel
  • if also showing it at other pages, then display it no more than a max of 3 times per session in non-control panel pages
@mbabker

This comment has been minimized.

Show comment
Hide comment
@mbabker

mbabker Aug 7, 2016

Member

I will change it to be consistent with the 2.5 EOS plugin to only show on the control panel.

I will NOT add a session counter.
I will NOT change the error type. I will add how the 2.5 EOS plugin is rendered as an error message as additional justification for this stance.

If that will preclude this being merged I'll just close this. I'm taking a stand on what I believe is the correct approach to this.

Member

mbabker commented Aug 7, 2016

I will change it to be consistent with the 2.5 EOS plugin to only show on the control panel.

I will NOT add a session counter.
I will NOT change the error type. I will add how the 2.5 EOS plugin is rendered as an error message as additional justification for this stance.

If that will preclude this being merged I'll just close this. I'm taking a stand on what I believe is the correct approach to this.

@nikosdion

This comment has been minimized.

Show comment
Hide comment
@nikosdion

nikosdion Aug 7, 2016

Contributor
Contributor

nikosdion commented Aug 7, 2016

@PhilETaylor

This comment has been minimized.

Show comment
Hide comment
@PhilETaylor

PhilETaylor Aug 7, 2016

Contributor

Just have balls and commit this without watering it down - listen to those of us that have to deal with the idiots on PHP 5.3 on a daily basis...

It should be in your face, in red and bold, on every page and un-dismissable.

Contributor

PhilETaylor commented Aug 7, 2016

Just have balls and commit this without watering it down - listen to those of us that have to deal with the idiots on PHP 5.3 on a daily basis...

It should be in your face, in red and bold, on every page and un-dismissable.

@ggppdk

This comment has been minimized.

Show comment
Hide comment
@ggppdk

ggppdk Aug 7, 2016

Contributor

If that will preclude this being merged I'll just close this.

this plugin / message is needed for reasons already discussed, nobody said not to add this

I will NOT add a session counter.

Only a suggestion if this is to be displayed in non-control panel pages

  • why i suggested it , because imagine that a website is upgraded to newer PHP e.g. after 2 days or 2 weeks, people will see it for 2days or 2 weeks in all backend pages ? while working in site's backend for hours everyday ? or did you say to only show it at control panel area ?
Contributor

ggppdk commented Aug 7, 2016

If that will preclude this being merged I'll just close this.

this plugin / message is needed for reasons already discussed, nobody said not to add this

I will NOT add a session counter.

Only a suggestion if this is to be displayed in non-control panel pages

  • why i suggested it , because imagine that a website is upgraded to newer PHP e.g. after 2 days or 2 weeks, people will see it for 2days or 2 weeks in all backend pages ? while working in site's backend for hours everyday ? or did you say to only show it at control panel area ?
@andrepereiradasilva

This comment has been minimized.

Show comment
Hide comment
@andrepereiradasilva

andrepereiradasilva Aug 7, 2016

Contributor

Ok, i see you all prefer the error aproach. since this is the current practice (2.5 EOL, joomla update notification, extension updates notification) is consistent, so fine by me.

@mbabker i have just one question, what happens when php 7.1 is released/used.

Contributor

andrepereiradasilva commented Aug 7, 2016

Ok, i see you all prefer the error aproach. since this is the current practice (2.5 EOL, joomla update notification, extension updates notification) is consistent, so fine by me.

@mbabker i have just one question, what happens when php 7.1 is released/used.

@@ -135,6 +135,7 @@ private function getPhpSupport()
// Check the PHP version's support status using the minor version
$activePhpVersion = PHP_MAJOR_VERSION . '.' . PHP_MINOR_VERSION;
+ $activePhpVersion = '5.3';

This comment has been minimized.

@wilsonge

wilsonge Aug 7, 2016

Contributor

Nope?

@wilsonge

wilsonge Aug 7, 2016

Contributor

Nope?

@mbabker

This comment has been minimized.

Show comment
Hide comment
@mbabker

mbabker Aug 7, 2016

Member

@mbabker i have just one question, what happens when php 7.1 is lanched/used.

The checks default to the PHP version being supported so messages aren't rendered for unknown versions. When a PHP version isn't found in the array, then it takes no action (we don't have data on the PHP version so we err on the side of caution and do nothing), specifically this line.

Member

mbabker commented Aug 7, 2016

@mbabker i have just one question, what happens when php 7.1 is lanched/used.

The checks default to the PHP version being supported so messages aren't rendered for unknown versions. When a PHP version isn't found in the array, then it takes no action (we don't have data on the PHP version so we err on the side of caution and do nothing), specifically this line.

@mbabker

This comment has been minimized.

Show comment
Hide comment
@mbabker

mbabker Aug 7, 2016

Member

because imagine that a website is upgraded to newer PHP e.g. after 2 weeks, people will see it for 2 weeks in all backend pages ?

It's a plugin, disable it if you're tired of Joomla nagging. At least I'm giving you that option and not hardcoding it directly into JApplicationAdministrator's constructor.

Member

mbabker commented Aug 7, 2016

because imagine that a website is upgraded to newer PHP e.g. after 2 weeks, people will see it for 2 weeks in all backend pages ?

It's a plugin, disable it if you're tired of Joomla nagging. At least I'm giving you that option and not hardcoding it directly into JApplicationAdministrator's constructor.

@andrepereiradasilva

This comment has been minimized.

Show comment
Hide comment
@andrepereiradasilva

andrepereiradasilva Aug 7, 2016

Contributor

ok thanks, will test faking the php versions.

Contributor

andrepereiradasilva commented Aug 7, 2016

ok thanks, will test faking the php versions.

@andrepereiradasilva

This comment has been minimized.

Show comment
Hide comment
@andrepereiradasilva

andrepereiradasilva Aug 7, 2016

Contributor

I have tested this item successfully on 3681966

works as described


This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/11498.

Contributor

andrepereiradasilva commented Aug 7, 2016

I have tested this item successfully on 3681966

works as described


This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/11498.

@ggppdk

This comment has been minimized.

Show comment
Hide comment
@ggppdk

ggppdk Aug 7, 2016

Contributor

It's a plugin, disable it if you're tired of Joomla nagging. At least I'm giving you that option and not hardcoding it directly into JApplicationAdministrator's constructor.

yes, after googling
"how i disable annoying message about php version in joomla",

they will find a web page with instructions to disable the plugin,
and then they will forget about it,
and that will defeat the purpose of upgrading the PHP

Too annoying messages are usually turned off and then forgotten

but i looked at the code you have already limitted the display to control panel,
which is very good, because

  • people will see it once at every login
  • now it is unlikely that they will disable the plugin !

in fact i think it is great now !!, i ll test

Contributor

ggppdk commented Aug 7, 2016

It's a plugin, disable it if you're tired of Joomla nagging. At least I'm giving you that option and not hardcoding it directly into JApplicationAdministrator's constructor.

yes, after googling
"how i disable annoying message about php version in joomla",

they will find a web page with instructions to disable the plugin,
and then they will forget about it,
and that will defeat the purpose of upgrading the PHP

Too annoying messages are usually turned off and then forgotten

but i looked at the code you have already limitted the display to control panel,
which is very good, because

  • people will see it once at every login
  • now it is unlikely that they will disable the plugin !

in fact i think it is great now !!, i ll test

@mbabker

This comment has been minimized.

Show comment
Hide comment
@mbabker

mbabker Aug 7, 2016

Member

but i looked at the code you have already limitted the display to control panel,

Which I think is the wrong move but politics seems to dictate that we can't be excessively annoying in telling users they should be responsible and update their stuff. ¯\_(ツ)_/¯

Member

mbabker commented Aug 7, 2016

but i looked at the code you have already limitted the display to control panel,

Which I think is the wrong move but politics seems to dictate that we can't be excessively annoying in telling users they should be responsible and update their stuff. ¯\_(ツ)_/¯

@wilsonge wilsonge added this to the Joomla 3.7.0 milestone Aug 7, 2016

+ *
+ * @since __DEPLOY_VERSION__
+ */
+ private function displayMessage()

This comment has been minimized.

@elkuku

elkuku Aug 7, 2016

Member

It looks strange that a display* method returns a boolean, I'd call it getDisplayStatus() maybe 😉

Otherwise: Great idea 100👍

@elkuku

elkuku Aug 7, 2016

Member

It looks strange that a display* method returns a boolean, I'd call it getDisplayStatus() maybe 😉

Otherwise: Great idea 100👍

@elkuku

This comment has been minimized.

Show comment
Hide comment
@elkuku

elkuku Aug 7, 2016

Member

I have tested this item successfully on 3681966


This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/11498.

Member

elkuku commented Aug 7, 2016

I have tested this item successfully on 3681966


This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/11498.

@dgrammatiko

This comment has been minimized.

Show comment
Hide comment
@dgrammatiko

dgrammatiko Aug 7, 2016

Member

I have tested this item successfully on 3681966


This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/11498.

Member

dgrammatiko commented Aug 7, 2016

I have tested this item successfully on 3681966


This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/11498.

@jeckodevelopment

This comment has been minimized.

Show comment
Hide comment
@jeckodevelopment

jeckodevelopment Aug 8, 2016

Member

I have tested this item successfully on 3681966

RTC Please


This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/11498.

Member

jeckodevelopment commented Aug 8, 2016

I have tested this item successfully on 3681966

RTC Please


This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/11498.

@joomla-cms-bot joomla-cms-bot added the RTC label Aug 8, 2016

@mbabker

This comment has been minimized.

Show comment
Hide comment
@mbabker

mbabker Aug 8, 2016

Member

The only thing that last commit does is change the method name per @elkuku suggestion. Shouldn't need re-testing.

Member

mbabker commented Aug 8, 2016

The only thing that last commit does is change the method name per @elkuku suggestion. Shouldn't need re-testing.

@andrepereiradasilva

This comment has been minimized.

Show comment
Hide comment
@andrepereiradasilva

andrepereiradasilva Aug 8, 2016

Contributor

I have tested this item successfully on f236d70

only method name changed.


This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/11498.

Contributor

andrepereiradasilva commented Aug 8, 2016

I have tested this item successfully on f236d70

only method name changed.


This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/11498.

wilsonge added a commit that referenced this pull request Aug 8, 2016

@wilsonge

This comment has been minimized.

Show comment
Hide comment
@wilsonge

wilsonge Aug 8, 2016

Contributor

Merged to 3.7.x with e4894d2 Thanks Michael!

Contributor

wilsonge commented Aug 8, 2016

Merged to 3.7.x with e4894d2 Thanks Michael!

@wilsonge wilsonge closed this Aug 8, 2016

@brianteeman

This comment has been minimized.

Show comment
Hide comment
@brianteeman

brianteeman Aug 11, 2016

Contributor

Doing the job of the lazy bot and removing the rtc flag

Contributor

brianteeman commented Aug 11, 2016

Doing the job of the lazy bot and removing the rtc flag

@joomla-cms-bot joomla-cms-bot removed the RTC label Aug 11, 2016

@mbabker mbabker deleted the mbabker:php-version-check branch Aug 11, 2016

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