Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Static versioning and styles minification break email fonts styles #8241

Closed
Igloczek opened this issue Jan 23, 2017 · 13 comments
Closed

Static versioning and styles minification break email fonts styles #8241

Igloczek opened this issue Jan 23, 2017 · 13 comments
Labels
Area: Frontend bug report Component: Customer Fixed in 2.3.x The issue has been fixed in 2.3 release line Issue: Format is valid Gate 1 Passed. Automatic verification of issue format passed Issue: Ready for Work Gate 4. Acknowledged. Issue is added to backlog and ready for development

Comments

@Igloczek
Copy link
Contributor

Preconditions

Enabled static versioning and styles minification in production mode.

Steps to reproduce

  1. Make an order or any other action that will send an email.
  2. Run static-content:deploy or make and application deployment
  3. Open previous email.

Expected result

  1. Your email should have proper styles including fonts (check console).

Actual result

  1. email-fonts.min.css generated by static-content:deploy isn't loaded, b/c email template is requesting for email-fonts.css. It's hardcoded in /web/css/source/_email-extend.less of Blank and Luma themes.
  2. Even if email-fonts.css will exist, running static-content:deploy or making an whole application deployment, will make path to this file unaccessible due to static version change.
@AirmanAJK
Copy link

My fix for this was to add "email" to the config setting 'dev/css/minify_exclude'. Whenever css filenames are generated, this config value (if present) is split by line break (\n) and compared with the file to potentially add ".min" to using a contains function. If any matches are found, no .min is added to the filename.

I had other problems with css and javascript files getting minified so that right now my two exclude configs look like this:

'dev/js/minify_exclude' : 'api/checkout\ntiny_mce\neditor'
'dev/css/minify_exclude' : 'tiny_mce\neditor\nemail'

This fixes several issues while minifying:

  1. Paypal's external checkout.js file (which has no min version and is downloaded remotely) being broken.
  2. Static email css imports are improperly referenced (as you've seen).
  3. TinyMCE intermittently going crazy with not finding extensions in the admin area.

An official fix here is tricky because these config values are a combination of settings from different areas, so an extension can't simply append it's own addition to it using the normal config files. You may be stuck with manually managing these settings in the database. Magento devs will probably have to rethink the exception lists for these resources, because there is a clear need for it.

@xmav xmav self-assigned this Feb 1, 2017
@xmav xmav added the Issue: Ready for Work Gate 4. Acknowledged. Issue is added to backlog and ready for development label Feb 2, 2017
@xmav xmav removed their assignment Feb 2, 2017
@xmav
Copy link
Contributor

xmav commented Feb 2, 2017

@Igloczek Thanks for reporting this issue.
We've created internal ticket MAGETWO-64085 to address this issue.

@xmav xmav added the 2.0.x label Feb 2, 2017
@PieterCappelle
Copy link
Contributor

Any update on this?

@tigerx7
Copy link

tigerx7 commented Apr 25, 2017

We just updated to 2.1.6 recently and ran into this problem. Even though we had CSS Minify set to false in admin, CSS was still being minified and breaking styles in the transactional emails. Digging into it, seems the issue is with the way the less parser is being used.

In https://github.com/magento/magento2/blob/develop/lib/internal/Magento/Framework/Css/PreProcessor/Adapter/Less/Processor.php#L73, the compress option is being set by if Magento is in developer mode or not... which clearly defies the purpose of the CSS minify option in admin since almost all CSS will be running through less.

We just removed the compress option from being passed on to less.php in Processor.php which resulted in non-minified CSS and correctly began styling the outgoing transactional emails.

@CNanninga
Copy link

Intrinsically related to the same cause is the fact that some inline styles are missing as well. Same factors (always happens when the forced minification takes place in production mode).

Normal email styles when static content deployment was done in developer mode: https://files.classyllama.com/cf99354a/screenshots/Your_Main_Website_Store_order_confirmation_-_chris.nanningaclassyllama.com_-_Classy_Llama_Studios_LLC_Mail_2017-07-27_09-23-15.jpg

The way it looks when deployment was done in production mode:
https://files.classyllama.com/cf99354a/screenshots/Your_Main_Website_Store_order_confirmation_-_chris.nanningaclassyllama.com_-_Classy_Llama_Studios_LLC_Mail_2017-07-27_09-59-39.jpg

In the second example, the style sheets were minified even though minification is disabled, and the missing styles are those that should be injected inline.

@xmav
Copy link
Contributor

xmav commented Aug 23, 2017

To fix issue just bump version of pelago/emogrifier to 1.2.0 in composer.json

@CNanninga
Copy link

@xmav - Thanks for the insight. Unfortunately, can't do that on an individual project, as the Magento Composer packages have 0.1.1 as a hard requirement. Hopefully we'll see this version constraint change in the core.

@xmav
Copy link
Contributor

xmav commented Aug 24, 2017

@CNanninga You can use composer alias to install other version.
Here is how it works for me:

  • update root composer.json and add "pelago/emogrifier": "1.2.0 as 0.1.1". Mine looks like after update:
"require": {  
        "magento/product-community-edition": "2.1.7",  
        "composer/composer": "@alpha",  
        "pelago/emogrifier": "1.2.0 as 0.1.1"  
    },  
  • remove old package "rm -rf vendor/pelago/emogrifier/"
  • install new version "composer update pelago/emogrifier"

@CNanninga
Copy link

@xmav - Spectacular. I'd forgotten about Composer aliases.

magento-team pushed a commit that referenced this issue Sep 1, 2017
magento-team pushed a commit that referenced this issue Sep 1, 2017
magento-team pushed a commit that referenced this issue Sep 1, 2017
…ak email fonts styles #8241

- fix adapter dependency not add collaborator in constructor to 3rd party libraries
magento-team pushed a commit that referenced this issue Sep 1, 2017
magento-team pushed a commit that referenced this issue Sep 1, 2017
magento-team pushed a commit that referenced this issue Sep 1, 2017
magento-team pushed a commit that referenced this issue Sep 1, 2017
magento-team pushed a commit that referenced this issue Sep 1, 2017
magento-team pushed a commit that referenced this issue Sep 4, 2017
…fonts styles #8241 #10638

 - Merge Pull Request #10638 from xmav/magento2:8241
 - Merged commits:
   1. 9f71bed
   2. 795b6b8
@magento-team magento-team added 2.1.x Issue: Ready for Work Gate 4. Acknowledged. Issue is added to backlog and ready for development bug report Component: Customer Area: Frontend labels Sep 4, 2017
@magento-engcom-team magento-engcom-team added 2.0.x Issue: Ready for Work Gate 4. Acknowledged. Issue is added to backlog and ready for development bug report Component: Customer Area: Frontend Issue: Format is valid Gate 1 Passed. Automatic verification of issue format passed and removed G1 Passed labels Sep 5, 2017
@magento-engcom-team magento-engcom-team added the Fixed in 2.3.x The issue has been fixed in 2.3 release line label Sep 13, 2017
@magento-engcom-team
Copy link
Contributor

Thank you @xmav for the fix

@CNanninga
Copy link

It's worth noting that I got a different solution from Magento Enterprise support, patch MDVA-4841, which contains core application code changes instead. Is there a consensus on which of these is better?

okorshenko pushed a commit that referenced this issue Nov 10, 2017
@ywsw
Copy link

ywsw commented Jul 2, 2018

I had this issue on 2.2.5. The static-content:deploy broke because it could not find email-fonts.css (without css min setting) or email-fonts.min.css (with the css min setting). This error also prevented lots of other stuff from getting compiled as well.

The file email-fonts.less definitely exists.

My solution (temporary) was to create an empty dummy email-fonts.css file in my theme. Once this was done the system compiled correctly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: Frontend bug report Component: Customer Fixed in 2.3.x The issue has been fixed in 2.3 release line Issue: Format is valid Gate 1 Passed. Automatic verification of issue format passed Issue: Ready for Work Gate 4. Acknowledged. Issue is added to backlog and ready for development
Projects
None yet
Development

No branches or pull requests

10 participants