User tests: Successful: Unsuccessful:
Pull Request for Issue #29108
This here just backports the new CssIdentifierRule to 3.10 so extensions can use the new validation rule in 3.10 too.
code review
Extension devs can use the new validation rule in 3.10 and 4.0
This behavior will be documented together with the 4.0 PR.
Status | New | ⇒ | Pending |
Category | ⇒ | Libraries |
Labels |
Added:
?
?
|
This is B/C breaking in the context of 3.x and how the module suffix class is actually used.
This is B/C breaking in the context of 3.x and how the module suffix class is actually used.
Why is this a B/C break? he only adds a class that's not used by the core in 3.10 and is intended for 3rd party developers to prepare there extension.
Anyone who uses it cannot use it in conjunction with the module class suffix as is used in 3.x, or other class suffix structures where there is no arbitrary leading space. The rule strips out a number of characters that would be used to indicate a modifier to the class instead of appending (i.e. with <div class="module<?php echo $suffix; ?>">
I cannot have $suffix = '--large'
as the rule indicates this BEM pattern is not allowed as the first characters, similar for $suffix = '-100'
or $suffix = '100'
).
This would be better left in 4.0 only instead of adding a class that adds some pretty hefty restrictions that would only cause end user confusion (why can I put this on X module but not on Y).
The rule strips out a number of characters that would be used to indicate a modifier to the class instead of appending (i.e. with
I cannot have $suffix = '--large' as the rule indicates this BEM pattern is not allowed as the first characters, similar for $suffix = '-100' or $suffix = '100').
Just to be clear that rule is not there in stone ;) So when we have the requirement to allow --large this can be added.
Do you have some more usecases we should take a look into?
Security or not, the remark in the code clearly states it will break existing sites no matter the UseCase.
Remark: “ // Identifiers cannot start with a digit, two hyphens, or a hyphen followed by a digit.”
Any site which upgrades to 3.10 and any change done by a user in a location where a class is used, which get denied by this check will break stuff.
This is not only irritating for the users, also for any site maintainer, developer whatsoever.
Not to mention the negative marketing it will generate with a possible (most likely) B/C break like this.
I’m not planning to fix sites where by any chance the class starts with -Digit, — or just digit.
@conconnl this introduces the validation rule but does not apply it to any existing fields in core. So it's completely optional in 3.10. The PR for 4.0 does apply the rule to all class fields.
But I agree, the regex needs to be changed to allow existing suffix uses. This applies to 4.0 as well. Even though core layouts no longer allows class suffixes, nothing should stop users and template developers from fixing this themselves.
Remark: “ // Identifiers cannot start with a digit, two hyphens, or a hyphen followed by a digit.”
Yes when you check where that code is based on you notice that they use is different that we do in our code.
So please let me know how you use this field and we make sure the validation rule supports that but we have to start somewhere.
This is not only irritating for the users, also for any site maintainer, developer whatsoever.
Not to mention the negative marketing it will generate with a possible (most likely) B/C break like this.
As mention above there is zero impact on 3.10 installations. ;)
But I agree, the regex needs to be changed to allow existing suffix uses. This applies to 4.0 as well. Even though core layouts no longer allows class suffixes, nothing should stop users and template developers from fixing this themselves.
Please add examples of what you use so we can make sure it passes the validation rule.
I'm going to close here as between 3.x and 4.x there are fundamentally changes in 4.x there is no sfx any more but in 3.x we usually have a sfx so better not port something back that can not be used (as in 3.x you use sfx) and would be different once updated to 4.x
Status | Pending | ⇒ | Closed |
Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2020-10-17 17:33:43 |
Closed_By | ⇒ | zero-24 |
I have tested this item✅ successfully on 497db9f
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/29109.