Problem/Motivation

Recent change in 7.x-dev causes style.css to be created incorrectly in color.module.

Steps to reproduce

1. Use garland theme.
2. Change to anything except blue lagoon.
3. It's easier to see if you have a sidebar enabled.
4. Note the sidebar color looks messed up in the lower half. In style.css a lot of entries are incorrectly `#000000`

Proposed resolution

I don't know but it's from https://git.drupalcode.org/project/drupal/-/commit/248892a6a0eae7d64d963...

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

CommentFileSizeAuthor
#6 3249605-6_test_only.patch1.69 KBmcdruid
#6 interdiff-3249605-3-6_test_only.txt1.2 KBmcdruid
#3 3249605-3_test_only.patch1.27 KBmcdruid

Issue fork drupal-3249605

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

demeritcowboy created an issue. See original summary.

demeritcowboy opened merge request !1430

mcdruid’s picture

mcdruid
Location 🇬🇧🇪🇺
CreditAttribution: mcdruid as a volunteer and at Acquia commented
FileSize
3249605-3_test_only.patch1.27 KB

Thanks for catching this.

I'd like to add a test along with the fix - here's a first pass.

Might be good to add a couple more sets of test values.

We expect the test_only patch to fail, but if we add the tests to the MR they should pass.

mcdruid’s picture

mcdruid
Location 🇬🇧🇪🇺
CreditAttribution: mcdruid as a volunteer and at Acquia commented
Status: Active » Needs review

NR for tests.

Status: Needs review » Needs work

The last submitted patch, 3: 3249605-3_test_only.patch, failed testing. View results

mcdruid’s picture

mcdruid
Location 🇬🇧🇪🇺
CreditAttribution: mcdruid as a volunteer and at Acquia commented
Status: Needs work » Needs review
FileSize
interdiff-3249605-3-6_test_only.txt1.2 KB
3249605-6_test_only.patch1.69 KB

Added some more test values, and have added the test to the MR.

Back to NR, but we expect this test-only patch to fail, whereas the MR should not.

Status: Needs review » Needs work

The last submitted patch, 6: 3249605-6_test_only.patch, failed testing. View results

longwave’s picture

longwave
he/him
Primary language English
Location UK
CreditAttribution: longwave at Full Fat Things commented
Status: Needs work » Reviewed & tested by the community

The test looks good and proves the fix works.

Hiding the test-only patch in an attempt to convince the bot that the MR is thing that is RTBC.

  • mcdruid committed a81b7bc on 7.x 
    Issue #3249605 by mcdruid, demeritcowboy: Fix regression in color module
mcdruid’s picture

mcdruid
Location 🇬🇧🇪🇺
CreditAttribution: mcdruid as a volunteer and at Acquia commented
Status: Reviewed & tested by the community » Fixed

Thank you!

demeritcowboy’s picture

demeritcowboy CreditAttribution: demeritcowboy commented

Thanks! Should I close the original merge request?

demeritcowboy closed merge request !1430

Status: Fixed » Closed (fixed)

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

mpp’s picture

mpp CreditAttribution: mpp at AmeXio for District09 commented

This fix removed the int conversion from https://www.drupal.org/i/3248752 which was a php 8.1 fix.

Re-added it in https://www.drupal.org/project/drupal/issues/3224299.

poker10’s picture

poker10 CreditAttribution: poker10 at ActivIT s.r.o. commented

@mpp I have commented that in the META issue, please check here: #3224299-65: [META] Make Drupal 7 core compatible with PHP 8.1