When _color_rewrite_stylesheet() compares color values, it normalizes the case but doesn't normalize equal versions of shorthand and longhand hex values.

Tests were breaking in #2866801: Update stylelint rules color-hex-length to be consistent with Drupal's CSS standards with shorthand CSS hex values.

Proposal

Convert to longhand to normalize and compare because it's easier.

CommentFileSizeAuthor
#6 2940088-6.patch3.38 KBjoelpittet
#6 interdiff.txt4.02 KBjoelpittet
#2 2940088-2.patch1.59 KBjoelpittet
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

joelpittet created an issue. See original summary.

joelpittet’s picture

joelpittet
Primary language English
Location Vancouver
CreditAttribution: joelpittet as a volunteer and at The University of British Columbia commented
Issue summary: View changes
Status: Active » Needs review
FileSize
2940088-2.patch1.59 KB

Here's a patch that fixed the broken tests in #2866801: Update stylelint rules color-hex-length to be consistent with Drupal's CSS standards.

joelpittet’s picture

joelpittet
Primary language English
Location Vancouver
CreditAttribution: joelpittet as a volunteer and at The University of British Columbia commented
Title: Shorthand hex colors break color module's comparison » Shorthand hex colors in CSS files break color module's comparisons
gapple’s picture

gapple
he/they
Primary language English
CreditAttribution: gapple as a volunteer commented
Status: Needs review » Reviewed & tested by the community
cilefen’s picture

cilefen CreditAttribution: cilefen as a volunteer commented
Status: Reviewed & tested by the community » Needs work

Thank you both. 

+++ b/core/modules/color/color.module
@@ -506,6 +506,27 @@ function color_scheme_form_submit($form, FormStateInterface $form_state) {
+function _color_normalize_hex_length($color) {

Rather than add more procedural code, maybe we should put this in Drupal\Component\Utility\Color—is there a reason not to?. Also, how about a unit test?

joelpittet’s picture

joelpittet
Primary language English
Location Vancouver
CreditAttribution: joelpittet as a volunteer and at The University of British Columbia commented
Status: Needs work » Needs review
FileSize
interdiff.txt4.02 KB
2940088-6.patch3.38 KB

@cilefen thanks, that's a good suggestion. Not sure if my naming skill is the best but it's pretty good I hope.

Here's a unit test and new method.

gapple’s picture

gapple
he/they
Primary language English
CreditAttribution: gapple as a volunteer commented
Status: Needs review » Reviewed & tested by the community

👍

  • catch committed d193229 on 8.6.x 
    Issue #2940088 by joelpittet, cilefen: Shorthand hex colors in CSS files...

  • catch committed 381e755 on 8.5.x 
    Issue #2940088 by joelpittet, cilefen: Shorthand hex colors in CSS files...
catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch at Third and Grove commented
Version: 8.6.x-dev » 8.5.x-dev
Status: Reviewed & tested by the community » Fixed

Committed/pushed to 8.6.x and cherry-picked to 8.5.x, thanks!

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.