User tests: Successful: Unsuccessful:
This moves the <strong> tag surrounding the language strings COM_SEARCH_SEARCH_KEYWORD_N_RESULTS and COM_SEARCH_SEARCH_KEYWORD_N_RESULTS_1 to the corresponding view file.
Disagree with this proposed change
for this change. Markup belongs in views or layouts, and not in language files, that is indisputable.
Actually, I see no need to add it to the view, though it must certainly be removed from language files. strong is not a stylistic element, is has a clearly defined semantic value, one which does not fit the use-case here.
In this example the output is neither serious nor urgent, so it appears strong is being used merely to add to the font-weight. That is incorrect, so I would recommend simply removing it from the language files.
@sovainfo
We have "strong" tags in various language strings, that's right. But most of them highlight a small part of text inside the string, not the whole string itself. This use can be discussed, too, but a HTML tag that has just the purpose to highlight the whole message should be in the view, not in the language file.
"no easy solution to do that in php" - For parts inside a language string, we have methods, too. In com_search, a span.badge is inserted inside exactly the string I'd like to change.
For the override: That's right, but first, you do need a view override for changing HTML tags in every other place. Why not here, too? If the template designer doesn't want a strong here, what can he do? Template designers have no access to core language files or language overrides, afaik. And second, I tend to agree with Seth...
@nternetinspired
The longer I think about this, the more I think it would be better to remove it completely. Though I'm sure we'll have at least 5 people here discussing about B/C within the next 10 hours, I don't think it's neccessary to have this strong here. I'm rather considering to put the string in a system message, because it is in fact a message for the user, that is placed above the content.
It already creates a B/C issue as is. So, it would only be for J4, why not do it properly. Document it and implement it core wide. I am not convinced this should be the coding standard, but when it is accepted, it is fine with me. Wonder whether it is worth all the work for extensions and languages.
Maybe I am being thick but how is this a B/C issue. Does it change the performance or function of anything
Wonder whether it is worth all the work for extensions and languages.
@sovainfo Can you elaborate how this would affect extensions? We are talking about a language string from com_search, used in a layout of that component.
Imho there are two possible cases after this PR is merged:
<strong><strong>
. I don't think that will cause any issue at all<strong>
tag. It will not break anything at all, it will just not be written bold anymore. It's a minor change in appearance, yes. But I would not call that a B/C break.Considering messages are allowed to be overridden, hardcoding something that was originally done in the message is creating an BC issue. Suppose someone went through the trouble to change the message to have only one word strong instead of all the words or removed the strong. He would need to revert the change.
We are not only talking about a change of two messages, we are talking about a change in coding standard. The suggestion is that strong doesn't belong in the message. It should be taken out and at the moment put somewhere else. Or use a different technique to get the same results.
Nothing against removing strong tags from these two messages, that would still respect existing overrides. Against the proposed coding standard of not allowing strong in a message!
Against the proposed coding standard of not allowing strong in a message!
What proposed coding standard?
The proposed coding standard is not allowing strong in the message. It is used as an argument for making this change. It is clear from the code the current standard is to allow strong in a message.
Considering messages are allowed to be overridden, hardcoding something that was originally done in the message is creating an BC issue. Suppose someone went through the trouble to change the message to have only one word strong instead of all the words or removed the strong. He would need to revert the change.
It's not hardcoded, it's in an overrideable layout. If someone wants to have only parts of the text strong, he could still do it but would need a layout override as well.
However I would still not call that a B/C break because nothing breaks.
Who has proposed this coding standard you are referring to? I don't see anything in the current coding standards about strong in messages either.
The only standard I can call to mind that is relevant is in the section semantics;
Use HTML according to its purpose.
http://joomla.github.io/coding-standards/?coding-standards/chapters/html.md
Anything that requires me to make changes to code because of changes that are no bugfixes is considered an BC issue by me.
You don't have to make changes to code, and this is a bug fix…
Only seen a request for code to be changed. That doesn't make it a bugfix. Sofar nobody explained the bug, only expressed their opinion on how you could get the same results differently. And I proved that wrong by pointing to the overlooked usage of a language override. Which force you to create a template override (making changes to code). Normally considered a BC issue.
This is not BC because it changes display and forces users to create a template override to get the same result they had before that PR. Should be closed.
@nternetinspired
We are allowing html markup in language strings generally speaking because they are useful, specially when parts of the value have to be highlighted, which is quite impossible to do otherwise.
In this case indeed the markup is around the whole value and should have been implemented in the layout.
Let's consider this for a major release imho
@sovainfo
Taking these two quotes...
Anything that requires me to make changes to code because of changes that are no bugfixes is considered an BC issue by me.
It already creates a B/C issue as is. So, it would only be for J4
...you're basically saying, we shouldn't implement any changes but bugfixes before J4?
Let's consider this for a major release imho
Afaik, the next release will be a major one (3.3), that's still possible. However, I don't know, why we should close this, but still consider it in the future. Let's leave it open and merge it for the next major release, that is afaik 3.3.
I agree with you, @infograf768 and @sovainfo: In some cases, HTML inside a language string can make sense. But in this case, it's surrounding the whole string, therefore it should be in the view. Otherwise, we could put the <p>
element that is around the message inside the string, as well.
I don't see any big B/C problem here. It could be discussed if we put the <strong>
tag to the view or leave it out completely. I think, I'd even prefer removing it completely, but that's just my opinion.
And as @nternetinspired wrote, the only "B/C break" would be something written bold or not bold anymore. If you look at css or layout changes, I'm sure you'll find several changes that are way bigger.
The next patch release is 3.2.4 and should contain only bugfixes, the next minor release is 3.3.0 and allows for bugfixes and new features. In no way these releases should create BC issues. In the past new features have been called bugfixes and technical/quality improvements as well. At this moment the minor release 3.5.0 will follow and won't allow new features. After that there will the next major release 4.0.0. changes that cause BC issues are allowed there.
As mentioned removing the strong tags would be acceptable to me, as a bugfix. That wouldn't cause a BC issue. Language override would still be respected. Only people disagreeing with the bugfix would have to create a language override.
Also agree with having strong here is a bad idea, putting the strong in php is even worse! The BC issue it creates is it nullifies the effort people may have put in to get the output they want. It is irrelevant that you think that can easily undone using a template override. BC means you don't have to do these things.
Also agree with finding changes way bigger. That doesn't mean you can let the small ones pass.
Speaking as a PLT member, this would not be considered a BC break for me because nothing would break at all. It doesn't break the layout or the design of a page. In the worst case it would show the whole sentence as strong when someone used a language override to only have it partially strong.
If we consider this a BC issue, we could change no HTML output at all, which makes no sense.
It's also technically a bug because a template can't change this appearance (eg not strong at all), which it should be able to do.
Hopefully your fellow PLT members can convince you of the contrary.
Thank you for explaining why this is actually perceived a bug, that is about the only positive mentioned.
I tend to agree on this not being a B/C break, as annoying as it may be for
some who want that statement in boldface. I don't think changing text from
bold to not or changing the color defining class of an alert box as a B/C
break personally if justified. From what I gather, this could be justified.
On Saturday, March 15, 2014, sovainfo notifications@github.com wrote:
Hopefully your fellow PLT members can convince you of the contrary.
Thank you for explaining why this is actually perceived a bug, that is
about the only positive mentioned.Reply to this email directly or view it on GitHub#3281 (comment)
.
Per definition this is a BC break. There is nothing you can do about that. In order to get the same results after the update I need to revert the so called fix or create an override. Per definition that is a break of BC.
Whether the impact is going to be as bad as with Legacy, DS, BCRYPT, just to name a few, I doubt that.
Consider this attitude an insult to its users.
Per definition this is a BC break
Which defintion? BC should refer to a public API which defines where BC applies and where not. To my knowledge this public API never was defined. I tried to settle that in the Google Group, but the response was quite low.
And I really doubt styling tags like <strong>
should be part of a public API.
I was referring to the more liberal usage of BC the joomla project has been using. Hope that that will continue to be used.
They already are part of the API, there is documentation on message files. Currently it is allowed to use html tags. When you manage to change the API and strip the html tags in a message, like textfilters and editors have done in the past, I'll boycot that and future releases.
I followed your efforts on that Google Group related to the public API. I am curious for your assesment on that. Maybe we can take that offline.
Currently it is allowed to use html tags.
Nobody said to disallow HTML tags in language strings. This PR is about two strings to remove the tags (and move them to the layout), and only those two. And most seem to agree that it would make sense here to remove the strong tags.
@infograf768 It is not this change, it is the principle. Too bad people still don't understand software release. One would expect them to learn from their mistakes. Small comfort that at least you understand!
Do you think, it would be better to leave the <strong>
out of the layout? If so, I can change this later, don't have much time at the moment, but in a few hours, I can do this.
That is correct, as I mentioned before, that takes away the BC break.
Ok, now the <strong>
is removed completely. Therefore, to get the same result as before, the user has to create a language override or a view override or add some font-weight: bold
to his CSS. So we have a small "BC break", that consists of taking the <strong>
away for everyone, but I doubt that many users will even notice this change.
Imho, this is the bigger BC than what we had before because it likely will affect more people.
Now it affects all users that don't have a language override for this strings already.
Originally it affected only those which had a language or a layout override.
Closed as per the comments above and on the tracker
Status | New | ⇒ | Closed |
Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2014-07-27 16:39:53 |
JoomlaCode item: http://joomlacode.org/gf/project/joomla/tracker/?action=TrackerItemEdit&tracker_item_id=33455&start=0