User tests: Successful: Unsuccessful:
The issue is that the version icons still appear, even though content history is off by default when upgrading from Joomla 3.1.5. The simple solution is to hide the icons by default if the feature is not enabled.
Special thanks to Michael for helping me find the code for the change and also for bouncing my idea(s) off of him for advice.
For more info, see discussions at:
http://joomlacode.org/gf/project/joomla/tracker/?action=TrackerItemEdit&tracker_item_id=32348
and:
http://joomlacode.org/gf/project/joomla/tracker/?action=TrackerItemEdit&tracker_item_id=32332
I don't think we want to by default disable versioning for everybody, and truth be told, even with my responses on that other PR I forgot it was there with everything going on. But, it does need to be consistent in at least the PHP because as it stands right now, upgraders will see the version toolbar button without the rest of the versioning checks being enabled.
I thought for new installs the settings will be saved using the SQL files. The default values in the config xml files should not matter there as far as I understand it. Or am I wrong here?
We're at such a point of inconsistency with our own code base, it's a massive headache to manage anything anymore...
So, yes, the core components all have a default config set up for install which basically invalidates every other default in the XML forms or PHP checks.
@mbabker I certainly agree with the statement about inconsistency :)
Imho, default values in PHP code and XML files should match everywhere to avoid unpredictable behaviours for updaters.
New installs are threated differently anyway as we set the (different) values there like we feel they are useful.
@Bakual Agree that PHP and XML defaults MUST match otherwise they get changed at next save!
Suggestion here: Why not by default ENABLE the versioning everywhere but just DISABLE globally by default until we are sure about B/C in 3.2.2... Then we can ENABLE default in global setting.
It's much simpler for people to turn it on, PLUS we can have it ON by default in new installs.
Thoughts?
See other PR why enabling it by default will not work.
@Bakual Yes, it needs to be consistent accross the board. It is a pain (see number of duplicate lines that need changes) because the param is not tested in an Observer centrally, and that all those layouts are not simply generated by the Observer centrally. Something that could be refactored in a future release.
Code review: Looks good, like a safe change, code-wise (as long as they are off by default ;-) ).