? Pending

User tests: Successful: Unsuccessful:

avatar smz
smz
8 Apr 2016

Pull Request for Issue #9491

Summary of Changes

This is a fix for part of what was found in #9491 and particularly the fact that options declared at the article level are disregarded.

Testing Instructions

for com_content:

  • Set the global option for "Show title" as "Hide"
  • In an article set the "Show title" option as "Show"
  • In an associated "Single article" menu item set "Show title" as "Use Global"
Without this patch:

Article title is not displayed

With this patch:

Article title is displayed

As a proof that options set at the menu level overrides what is declared at the global and article level, now set "Show article" as "Hide" in the menu item: title is not displayed.

avatar smz smz - open - 8 Apr 2016
avatar smz smz - change - 8 Apr 2016
Status New Pending
avatar joomla-cms-bot joomla-cms-bot - change - 8 Apr 2016
Labels Added: ?
avatar smz smz - change - 8 Apr 2016
Title
Fixes articles view parameters hierarchy
Fixes articles options hierarchy (precedence)
avatar brianteeman
brianteeman - comment - 8 Apr 2016

Is that not the expected behaviour


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

avatar Fedik
Fedik - comment - 8 Apr 2016

by your testing instruction I would expect that in the result the title will be shown (not hidden),
as it set for the particular article (that override "Article Default")

Expected hierarchy is:
Global
Article item (override Global)
Menu item (override Article item)

avatar smz
smz - comment - 8 Apr 2016

Expected hierarchy is:
Global
Article item (override Global)
Menu item (override Article item)

I totally agree with you, 100%, @Fedik, but this is not the current behaviour: it will be once this patch is applied!

avatar Fedik
Fedik - comment - 8 Apr 2016

ah, I see ... I misunderstood your description

avatar smz
smz - comment - 8 Apr 2016

@Fedik no problem!

To make things clearer, here is a logical combinations table which describes what happens when changing an article option (e.g.: "Show Title") at the global (component) level, at the article level and at the menu item (single article) level. The table is not exhaustive, but it gives an idea...

T = Show
F = Hide
X = Use Global

Global Article Menu Current Expected
T X X T T
T X F F F
T F X T F
T F T T T
F X X F F
F T X F T
avatar SharkyKZ
SharkyKZ - comment - 8 Apr 2016

"Use global" in menu item options should be renamed to "Use article settings". It will be misleading otherwise.

avatar Bakual
Bakual - comment - 8 Apr 2016

Keep in mind that it depends what type of menu item you have. If we have a single article menu item, and the article set in the menu item matches the article displayed, then the menu item takes precedence over the article settings.
If itis a different menu item (eg category view) or the article doesn't match the menu item, then the article settings take precedence.

So assuming you're testing with a single article menu item, then the table shows the expected behavior and your expectations are wrong :smile:

avatar Bakual
Bakual - comment - 8 Apr 2016

"Use global" in menu item options should be renamed to "Use article settings". It will be misleading otherwise.

No, because it would be wrong as well :smile:

avatar SharkyKZ
SharkyKZ - comment - 8 Apr 2016

@Bakual, article menu item's default values should be inherited from article rather than global because article is meant to override global. Currently, if you set options in article and create a menu item, you have to explicitly set the options again in menu item because "Use global" will inherit global options. Actually, "Use global" setting is currently correct because it does what it says, but it would be much better if the menu item would inherit article settings.

avatar brianteeman
brianteeman - comment - 8 Apr 2016

Irrelevant on any understanding of if the current or proposed priority is
correct how is it possible to make any change that will not change peoples
web sites on an update

On 8 April 2016 at 13:56, SharkyKZ notifications@github.com wrote:

@Bakual https://github.com/Bakual, article menu item's default values
should be inherited from article rather than global because article is
meant to overrides global. Currently, if you set options in article and
create a menu item, you have to explicitly set the options again in menu
item. Currently, "Use global" setting is actually correct, but it would be
much better if the menu item would inherit article settings.


You are receiving this because you commented.
Reply to this email directly or view it on GitHub
#9801 (comment)

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

avatar smz
smz - comment - 8 Apr 2016

@Bakual

Keep in mind that it depends what type of menu item you have. If we have a single article menu item, and the article set in the menu item matches the article displayed, then the menu item takes precedence over the article settings.

agreed.

If itis a different menu item (eg category view) or the article doesn't match the menu item, then the article settings take precedence.

In "Category List" we don't display articles. We display articles in "Category Blog" view, where we have too (as in "Single article") "article options" to set. If we set an option there good, we should use it. If we set "Use global", the choice should "bubble up" to the article level and if not set there either we go to the global level. Right?

May I ask which is the rarest of the rarest case where options set at the article level are today taken into account? I failed to find one...

avatar smz
smz - comment - 8 Apr 2016

@SharkyKZ

"Use global" setting is currently correct because it does what it says, but it would be much better if the menu item would inherit article settings.

... well, yes, if you interpret "Use global" as "Use what is set at the global level (and forget about the article level)", but I think this is a stretch...

avatar designbengel designbengel - test_item - 9 Apr 2016 - Tested successfully
avatar designbengel
designbengel - comment - 9 Apr 2016

I have tested this item :white_check_mark: successfully on 94f9ccc


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

avatar Bakual
Bakual - comment - 9 Apr 2016

"Use Global" is meant to use the global component options. It is not meant to take the article options at all. So this PR would be wrong imho and it would change existing behavior.
One could add a "Use Article Setting" option, but it would be quite pointless in that view (just set it in the menu item). That's why it isn't there. The "Use Article Setting" is available for example in the menu item for the category blog, where it makes more sense.

avatar smz
smz - comment - 9 Apr 2016

@Bakual

... I don't know... I want to think it over a little bit better, but in the next days I'll be quite busy, so let us keep this in the fridge for a while... OK?

avatar smz
smz - comment - 10 Apr 2016

@Bakual and everybody:

I think I have a point (my guts where telling me that there was something wrong in all of this...), but please, try following my reasons abstracting from what current behaviour is, any B/C related problem and what the code/data structures are. Those are "2nd level" issues for which we can devise a solution or - in case we really can't address them now - we will accept the situation as it is and we'll just take this as an exercise for what probably we should do at the earliest possible occasion.

Let me tell you a story:

Imagine an NGO has a Joomla "blog", a "webzine", about some delicate matters, maybe the women condition in some underdeveloped country, or political matters under a dictatorship, something like that.

The NGO give access to the webzine to its contributors and, as a policy, requires that every contributor has his/her personal account because the authorship of the articles must be somehow internally traceable

But there are cases when authors must hide their identity to the public (probably for very good reasons).

Contributors are very well taught that when composing an article they can set the "Display author" option to "Hide" if they deem that necessary, and the webmaster has correctly set-up the Blog to "Use article settings". Bingo! Author is hidden.

Time passes and the NGO decide that some of those article must be "archived" and they move them to the "Archive" section which of course is implemented using Joomla "Archived articles" feature.

Oops... "Archived articles" doesn't have "Use article settings" and is set at its default of "Use global setting", the webmaster has changed, doesn't think of the implications of what he's doing, et voilà, the identity of some whistle-blowers authors is revealed (and adieu to some of them...)

So...

Not only we must have a very well system-wide implemented hierarchy of options that are always applied in a predictable manner, but I changed my mind and I now think that the correct order of the hierarchy should be different, that is, in descending order of priority:

  1. Content (article) level
  2. Menu item level
  3. Global level

Content is king, not the webmaster (not to talk about us, developers...)

Of course there are cases when it would be better not exposing some options to authors/editors/publisher, but that should be solved at the ACL level.

This, I think, is particularly important in Joomla, where, due to the very well known routing limitations, content could end up displayed in an unpredicted "view".

Again: content must be king. Please think about this, then we can talk about the technicalities... (but I already have some rough idea...)

avatar smz
smz - comment - 10 Apr 2016

Pinging @chrisdavenport as the above considerations might have some implication with the implementation of "fields"...

avatar chrisdavenport
chrisdavenport - comment - 10 Apr 2016

Given any fixed precedence hierarchy, I'm sure you'll always be able to find (edge) cases where you want something different. So the options are:

  • Change the precedence hierarchy. This breaks websites and merely shifts the problem to a different set of cases.
  • Add more UI options to allow the user to determine the order of precedence in some way. There are different ways of doing that but I think that all will lead to more confused users, in degree as well as numerosity.
  • Leave it as it is and allow those who encounter such cases to apply their own rules in layout overrides.

At the moment, I tend to favour the last of these.

What implications do you foresee for the implementation of custom fields?

avatar Bakual
Bakual - comment - 10 Apr 2016

@smz: We already have a system which is to much complex. As we see, it's confusing for people.
With J4 I would rather remove some of that logic, but we can't change it in J3 as it would certainly change sites (like your NGO one).

avatar smz
smz - comment - 10 Apr 2016

@chrisdavenport Hi, Chris,

starting from the bottom:

What implications do you foresee for the implementation of custom fields?

If (sorry but I still have to look at them) in custom field there will be a similar mechanism for choosing what to display (at the article/menu item/global levels), then the same considerations could apply.

Given any fixed precedence hierarchy, I'm sure you'll always be able to find (edge) cases where you want something different

Probably, but well thought algorithms + data helps a lot in giving flexibility: think "regular expressions"

  • Change the precedence hierarchy. This breaks websites and merely shifts the problem to a different set of cases.

acknowledged, but it probably could boil down to having a single option (at the global level) for choosing which hierarchical model to use: old/new, with default set to "old".

  • Add more UI options to allow the user to determine the order of precedence in some way. There are different ways of doing that but I think that all will lead to more confused users, in degree as well as numerosity.

Same as above.

  • Leave it as it is and allow those who encounter such cases to apply their own rules in layout overrides.

That's not feasible: once you are in the layouts, $item->params has already been "treated" at the JLegacy model and at the component view levels and you have lost some information.

Beside that, I think it is correct to have the minimum needed information exposed in templates/layouts, so that template/layout designers cannot screw up (too much...) the "business logic" applied at the higher level. This, I think, is the beauty of JLayouts, in which (when correctly used and not injected with the MVC $this object) you have access only to the needed information, i.e.: "what is to be displayed" ($displayData).

avatar Bakual
Bakual - comment - 10 Apr 2016

having a single option (at the global level) for choosing which hierarchical model to use: old/new, with default set to "old".

Please no. That would be a nightmare codewise. We should make it less complex (probably earliest with J4 due to B/C break), not add another layer of complexity to it.

avatar smz
smz - comment - 10 Apr 2016

... That would be a nightmare codewise. ...

As I said, at this stage I'm not much concerned about the technicalities, but more about doing "the right stuff".

In any case I have the strong feeling that it will be 2 or 4 statements max (excluding the "field" in the xml and the language strings...)

avatar brianteeman
brianteeman - comment - 10 Apr 2016

As any change to this will break B/C so cant be merged why even waste time
talking about how complex or not the code is

avatar brianteeman
brianteeman - comment - 12 Apr 2016

Setting to Needs Review so the maintainers can make a decision


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

avatar brianteeman brianteeman - change - 12 Apr 2016
Status Pending Needs Review
avatar brianteeman brianteeman - change - 13 Apr 2016
Category Components
avatar Kubik-Rubik
Kubik-Rubik - comment - 7 May 2016

Thank you for creating this, @smz. At this time it can not be accepted as it would break our backwards compatibility promise but I am marking this to be re-evaluated for Version 4 which is the next release where we can make these breaking changes.

avatar Kubik-Rubik Kubik-Rubik - change - 7 May 2016
Status Needs Review Closed
Closed_Date 0000-00-00 00:00:00 2016-05-07 16:10:15
Closed_By Kubik-Rubik
avatar Kubik-Rubik Kubik-Rubik - close - 7 May 2016
avatar Kubik-Rubik Kubik-Rubik - close - 7 May 2016

Add a Comment

Login with GitHub to post a comment