Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.As a result of this bug report there is now a new bug.
For those of us that use gulp/grunt for sass and js building without use of preprocessor within Drupal this module now loads source sass, which obviously doesn't work. It would make sense to check for SASS/LESS preproecssor module before adding sass/less to the file scan.
if (module_exists("sass") || module_exists("sassy")) {
$mailstyles = file_scan_directory($themepath, '#^mail(-.+)?\.(c|sc|sa)ss$#');
} else if (module_exists("less") || module_exists("twbs_less")) {
$mailstyles = file_scan_directory($themepath, '#^mail(-.+)?\.(c|le)ss$#');
} else {
$mailstyles = file_scan_directory($themepath, '#^mail(-.+)?\.css$#');
} else
| Comment | File | Size | Author |
|---|---|---|---|
| #7 | 2844299-Incorrectly-includes-unprocessed-styles-7.patch | 945 bytes | sjerdo |
| #5 | 2844299-Incorrectly-includes-unprocessed-sass-less-scss-etc.patch | 1.32 KB | xrxphawxby |
| Capture.PNG | 14.5 KB | xrxphawxby | |
Comments
Comment #2
xrxphawxby CreditAttribution: xrxphawxby commentedFix included that doesn't require lots of module_exists tests
Comment #3
xrxphawxby CreditAttribution: xrxphawxby commentedComment #4
xrxphawxby CreditAttribution: xrxphawxby commentedI was clearly a little too eager the first time I wrote this patch. *sigh*
Comment #5
xrxphawxby CreditAttribution: xrxphawxby commentedComment #6
lukedekker CreditAttribution: lukedekker commentedJust ran into this issue when updating to the latest version of MimeMail. Assuming that the user is making use of a preprocessor seems like a horrible idea.
Comment #7
sjerdoExperienced the same issue. A .scss files was loaded instead of the .css file.
Updated the patch.
Comment #8
dmsmidtTested, works like a charm. Patch also confirms to our coding standards.
Comment #9
dmsmidtMarking this as critical since all mail output breaks.
Comment #10
lukedekker CreditAttribution: lukedekker commentedJust verified that this issue is still present against 7.x-1.1 and that the patch still applies cleanly and works properly.
Bumping the relevant version.
Comment #11
Lukas von BlarerCould we get this in?
Comment #12
TR CreditAttribution: TR commentedThe patch in #5 was well commented, but introduced a number of coding standards problems that need to be addressed.
The patch in #7 didn't have any coding standards problems, but it didn't do a good job of explaining in the comments what it was doing and why.
Also, both #5 and #7 assume there will be only mail.css, mail.less, mail.sass, and/or mail.scss present. But the original feature request that we're trying to fix here, #2146513: Scan Theme for other *css* file types (Preprocessors), also introduced the use of multiple style sheets named mail-*.css, etc. The patches in #5 and #7 ignore these additional style sheets.
We need a new patch to address these points. And it would be really nice to have a test to ensure that the expected style sheets are located properly by this module. If we had a test case for #2146513: Scan Theme for other *css* file types (Preprocessors) this bug may not have been introduced. And if we had a test case as part of #5 or #7 it might have caught the fact that the additional mail-*.css stylesheets were being missed.
(The issue queue priority guidelines indicate that this is clearly a "Normal" priority bug. Please read the guidelines, linked below, before resetting the priority.)