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

Can't parse @font-face nested in @media. #102

Closed
jonkemp opened this issue Oct 13, 2019 · 3 comments · Fixed by #103
Closed

Can't parse @font-face nested in @media. #102

jonkemp opened this issue Oct 13, 2019 · 3 comments · Fixed by #103

Comments

@jonkemp
Copy link
Contributor

jonkemp commented Oct 13, 2019

Breaks on code like this.

@media screen {
    a {
        color: blue !important;
        background: red;
    }
    @font-face {
        font-family: 'Arial2';
    }
}
@NV NV closed this as completed in #103 Nov 11, 2019
NV pushed a commit that referenced this issue Nov 11, 2019
@NV
Copy link
Owner

NV commented Nov 13, 2019

I unrolled this because it broke nested media queries #104.

CSSOM/lib/parse.js

Lines 389 to 415 in a4b46da

// Handle rules nested in @media or @supports
hasAncestors = ancestorRules.length > 0;
while (ancestorRules.length > 0) {
parentRule = ancestorRules.pop();
if (
parentRule.constructor.name === "CSSMediaRule"
|| parentRule.constructor.name === "CSSSupportsRule"
) {
prevScope = currentScope;
currentScope = parentRule;
currentScope.cssRules.push(prevScope);
break;
}
if (ancestorRules.length === 0) {
hasAncestors = false;
}
}
if (!hasAncestors) {
currentScope.__ends = i + 1;
styleSheet.cssRules.push(currentScope);
currentScope = styleSheet;
parentRule = null;
}

This logic looks rather complex. I'm not going to attempt to fix it right now.

I'll still accept a patch that handles both nested media queries and @font-face rules inside of media queries.

@NV NV reopened this Nov 13, 2019
@NV
Copy link
Owner

NV commented Nov 14, 2019

@devrelm since you implemented nested media rules, perhaps you know how to fix parsing of @font-face nested in @media properly.

@NV NV closed this as completed in 71d0798 Nov 14, 2019
@jonkemp
Copy link
Contributor Author

jonkemp commented Nov 14, 2019

@NV thanks for fixing!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants