[5.2] Allowing menu items to have multiple 'rel' values

[crystalenka]

Summary of Changes

According to MDN, the rel attribute on <link> and <a> elements can have multiple space-separated values.

This PR makes this a little easier by turning the rel field into a fancy select list and allows multiple values, which get imploded into a string before getting passed back to the layout (so there shouldn't be B/C problems).

Testing Instructions

For this PR to be a success a few things must be true:

1. Existing rel fields must be converted over without loss of data.
2. You must be able to add multiple link type values and have them show up on the link in the front end.

To test:

1. BEFORE applying the patch, make a menu item of type URL and go to the "Link Type" tab. Select a link type from the "link rel attribute" dropdown.
2. Go to the front end and make sure the link is displaying correctly, with the selected attribute getting rendered in the HTML. (It should look something like <a href="url here" rel="selected-attribute">.)
3. Apply the patch.
4. Success criteria 1: Go to the front end and make sure the link is still displaying the same way as before.
5. Go back to the link type settings and select multiple attributes. Save.
6. Success criteria 2: Go to the front end and make sure the selected attributes display in the html, separated by spaces. (It should look something like <a href="url here" rel="selected-attribute another-attr attribute-three">.)

Actual result BEFORE applying this Pull Request

Could not select multiple rel attributes.

Expected result AFTER applying this Pull Request

Existing links are unchanged; new capability of selecting multiple rel attributes works well.

Link to documentations

Please select:

• Documentation link for docs.joomla.org:

• No documentation changes for docs.joomla.org needed

• Pull Request link for manual.joomla.org:

• No documentation changes for manual.joomla.org needed

I don't know yet whether documentation is needed.

[brianteeman]

Must admit to never having seen this before. But following your links for adding missing values I see other values we have in our list that are not valid

actually its just prefetch that isdwrong. The others that I thought were invalid are non standard values created by google and should be included

[crystalenka]

Must admit to never having seen this before. But following your links for adding missing values I see other values we have in our list that are not valid

actually its just prefetch that isdwrong. The others that I thought were invalid are non standard values created by google and should be included

Yes, I saw prefetch too (which doesn't actually do anything when it's on an <a> tag) but not sure if we can straight up remove it without breaking b/c.

[gunjanpatel]

I don't remember in Joomla Framework we have some method to join an array but it would be nice if we have something like,

$rel = ArrayHelper::join($itemParams->get('menu-anchor_rel'), ' ');

Then this code would have been very simple to read. 🤔

[crystalenka]

The issue is that I'm also checking if it's an array or not, because prior values would be a string. I'm going to be looking if there's a better way to solve this though without breaking b/c.

[gunjanpatel]

I 100% agree with your code change in this PR. I was trying to suggest that we may do something better by adding a generic class/method to avoid this kind of duplication. Also if we sniff our codebase there might be many places where it can be improved. :)

[SharkyKZ]

Rebase on 5.0.

[crystalenka]

Rebase on 5.0.

Can I ask why? It's not a massive change or backwards compatibility break unless I've missed something. Really very minor change, there are bigger ones going into 4.3.

[SharkyKZ]

Rebase on 5.0.

Can I ask why? It's not a massive change or backwards compatibility break unless I've missed something. Really very minor change, there are bigger ones going into 4.3.

You missed the part where you had to make amends in the login module for it not to crash. Extension developers must not be required to make changes in patch and minor versions. But you're right in that many more breaking changes have been introduced lately. It's almost as if the project does not follow SemVer anymore and screwing over Joomla users is the norm. So it's fine, another B/C break that bricks sites in a patch release is fine 🤡.

[crystalenka]

Snark is unnecessary, it was a genuine question. Please be kind.

I didn't make any changes to the login module.

I made a very minor change to the menu module helper, which still works with data saved prior to this change. (Hence my test case 1.)

Probably I could do it more elegantly as evidenced by the review comments, but I genuinely don't see how this is a breaking change when there is a check for the data type and it can handle both ways.

[crystalenka]

Okay, so, I understand better now how the changes I suggested are a b/c break.

Is there a way to approach this that would allow multiple rel values without the b/c break? Earlier in the process maybe, so that it gets turned to a string before any 3rd party component or plugin or template would call them?

I thought the menu helper would be early enough, but I understand some people may bypass it.

[ReLater]

Copied from #39275 on request

ReLater:
In terms of maintenance and usability in other extensions, wouldn't it be better to introduce a new form field in Joomla? RelAttributesField.php or something?

crystalenka:
And do what with the input/data/current field? Wouldn't that be a bigger b/c break?

ReLater:
the idea is instead of using

<field
 name="menu-anchor_rel"
 type="list"
 label="COM_MENUS_ITEM_FIELD_ANCHOR_REL_LABEL"
 default=""
 validate="options"
>

to use

<field
 name="menu-anchor_rel"
 type="RelAttributes"
 label="COM_MENUS_ITEM_FIELD_ANCHOR_REL_LABEL"
 default=""
 validate="options"/>

You would have a single location to change/add/(remove) new options.

RelAttributesField.php could be a form field that extends ListField and thus would inherit all attributes of list field and behavior (multiple, layout etc.).

Concerning B\C break: I have absolutely no idea if that is one. The idea with a new field.

Was just an idea because I saw that you was correcting the options in 2 files.

[HLeithner]

This pull request has been automatically rebased to 5.0-dev.

[HLeithner]

This pull request has been automatically rebased to 5.1-dev.

[HLeithner]

This pull request has been automatically rebased to 5.2-dev.