In the original discussion that lead to the addition of layout_attributes_merge, swentel seemed to agree to have it enabled by default: #1605212-6: Template attributes missing from Display Suite layouts. The patch was committed as such but since then this option default has been change to disabled, which is causing the default behavior of DS to discard the RDFa attribute on the node wrapper. See issue in recipe module and schema.org for example: #1811750: Recipe rich snippet no longer working.
I've tracked the changes down to two commit:
1. 2d9a79 from #1787576: Field template settings save to much info, however none of the patches from that issue include any hunk to make that option default to FALSE, so I suspect this is an unintentional change.
2. 5c07ff which is not linked to any issue.
Were these changes intentional? I think the default behavior of Display Suite should be not to break the markup that Drupal core generate, don't you agree?
| Comment | File | Size | Author |
|---|---|---|---|
| #6 | 1822274-6.patch | 1.59 KB | swentel |
| #5 | 1822274_4_ds_a_merge_true.patch | 1.59 KB | swentel |
| #1 | 1822274_1_ds_a_merge_true.patch | 1.51 KB | scor |
Comments
Comment #1
scor commentedhere is a patch to resets layout_attributes_merge back to enabled by default.
Comment #2
swentel commentedHmm, yeah I seem to reverted that behavior during refactoring of the field template settings bug. Let me recheck with the tests, I agree this should be default. I might create a variable from this, defaulting to TRUE (only controllable through $conf, not UI), because I know some frontend people at work complained they had not idea what this actually does and then toggled it to remove the behavior (although in a lot of cases it likely does nothing).
Comment #3
scor commented@swentel great! variable only should work too, that way we would be sure people can't screw up their attributes and turn it off by mistake.
do you think there should be an update function to turn it back on on existing sites running ds 2.0, or do you think it could lead to some unexpected results? (happy to write the update function if you think it's a good idea).
Comment #4
swentel commentedI think it might end up in unexpected results - also, I don't think that many sites run 2.0 at this point, so I'd leave it as is for now, especially because 2.1 is going to follow quite soon as there's a annoying caching bug for custom ds fields. So this one will go on before monday or tuesday and then release after that.
Comment #5
swentel commentedActually, went in already, with the variable, thanks for the reminder!
Comment #6
swentel commentedActually, prefixed the variable with 'ds_' just to be sure.
Comment #7
swentel commentedMoving to 8.x so I don't forget it there.
Comment #8
swentel commentedCommitted and pushed.