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

Prod config produces invalid font-family values in minified CSS #4121

Closed
veltman opened this issue Mar 7, 2018 · 4 comments · Fixed by #8106
Closed

Prod config produces invalid font-family values in minified CSS #4121

veltman opened this issue Mar 7, 2018 · 4 comments · Fixed by #8106

Comments

@veltman
Copy link

veltman commented Mar 7, 2018

Is this a bug report?

Yes

Steps to Reproduce

  1. Import a stylesheet with a font-family declaration that includes a quoted, multiple-word font name that includes a generic CSS font family keyword as its first word (monospace, cursive, serif, etc.). For example:
body {
  font-family: "Monospace Number";
}
div {
  font-family: "Primary Font", "Cursive Font";
}
  1. Build a minified bundle with npm run build.

Expected Behavior

The font declarations are intact.

Actual Behavior

  1. cssnano's minification will drop the quotes, leading to:

body{font-family: Monospace Number}div{font-family: Primary Font,Cursive Font}

These are both invalid font-family value because of the keyword + lack of quotes. Confirmed they get ignored as invalid in Chrome 64, Firefox 58, and Safari 11.

Related issues

cssnano/cssnano#434
cssnano/cssnano#439

Suggested fix

I suspect this comes up a fair amount because "Monospace Number" is used throughout some versions of the Ant Design stylesheet. It may also affect other weird name scenarios, like unicode characters in a font name or, the edgiest of edge cases, someone who used a custom font named, e.g., "serif" (I believe the spec allows this if it's quoted!).

Given that it's unclear whether cssnano will change its default behavior, this could be fixed by updating webpack.config.prod.js to pass an option through to preserve the quotes:

// Old
minimize: true,

// New
minimize: { minifyFontValues: { removeQuotes: false } },

If this sounds reasonable I can send a PR.

@Timer Timer added this to the 2.0.0 milestone Mar 7, 2018
@Timer
Copy link
Contributor

Timer commented Mar 7, 2018

This sounds reasonable.

@maxekman
Copy link

maxekman commented Oct 3, 2018

I have also run into this problem with Ant Design for a production build. Would be great to get this merged.

@Timer Timer removed this from the 2.x milestone Nov 2, 2018
@stale
Copy link

stale bot commented Dec 2, 2018

This issue has been automatically marked as stale because it has not had any recent activity. It will be closed in 5 days if no further activity occurs.

@stale stale bot added the stale label Dec 2, 2018
@stale
Copy link

stale bot commented Dec 8, 2018

This issue has been automatically closed because it has not had any recent activity. If you have a question or comment, please open a new issue.

@stale stale bot closed this as completed Dec 8, 2018
@Timer Timer reopened this Jan 8, 2019
@stale stale bot removed the stale label Jan 8, 2019
@Timer Timer added this to the 2.1.4 milestone Jan 8, 2019
@ianschmitz ianschmitz modified the milestones: 2.1.4, 2.1.5 Feb 10, 2019
@iansu iansu modified the milestones: 2.1.6, 2.1.x Mar 6, 2019
@iansu iansu modified the milestones: 2.1.x, 3.x Mar 10, 2019
@lock lock bot locked and limited conversation to collaborators Dec 14, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
5 participants