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

Error deprecated ie6/7 properties syntax #85

Merged
merged 3 commits into from
Jan 27, 2022
Merged

Conversation

shagkur
Copy link
Contributor

@shagkur shagkur commented Jan 25, 2022

No description provided.

@phax
Copy link
Owner

phax commented Jan 25, 2022

Thanks @shagkur Will look at it.
The main reason I was a bit reluctant is the unambiguity when using "calc". a*b is no longer resolvable....

@phax
Copy link
Owner

phax commented Jan 25, 2022

But your proposal to use it for the property only.... If that works for you....
Shall we make a "browser compliant" variation? Shoud we eventually insert a "legacy mode"?

@shagkur
Copy link
Contributor Author

shagkur commented Jan 26, 2022

Well the thing is, for example following style declaration block:

div {
max-width: 100px;
*zoom: 1;
position: relative;
}

will result in

div {
max-width: 100px;
}

Which is wrong. At least in browser compliant mode. Just to give you the context to my issue.
However, the proposed solution works for me, because i only use it in the property, as you said. BUT: I'm nowhere sure this is the right way to do so, especially about the parser syntax i use here. That's why i'm quite happy that you took a look and maybe you have a better and cleaner solution in mind.
Currently, when the parser hits *zoom it is not detected as an IDENT and hence will finally throw the ParseException instyleDeclarationBlock() because the parsed token does not match the expected one (RBRACE then). And hence will drop everything until the RBRACE.
I don't think there's a need for something like "legacy mode", supporting these IE hacks again. But as i wrote in the issue report, dropping (in browser compliant mode) for these properties in question, should only happen on that specific property.
Btw. what do you mean by "browser compliant" variation?

@shagkur
Copy link
Contributor Author

shagkur commented Jan 27, 2022

@phax Any opinion on my last comment?

@phax
Copy link
Owner

phax commented Jan 27, 2022

Opinion: yes - yet time: unfortunately not yet

@shagkur
Copy link
Contributor Author

shagkur commented Jan 27, 2022

@phax No worries. :) I'll stick, in my project, with these changes since they work for me. But i'd be happy to discuss/work out this more detailed when you get some time to do so :)
I'll leave this PR open.
Would that work for you?

@phax
Copy link
Owner

phax commented Jan 27, 2022

Yes sure - I just need to buy me some time :)

@shagkur
Copy link
Contributor Author

shagkur commented Jan 27, 2022

@phax How about once in the evening? (j/k) 🥲

@phax phax merged commit 9bb0c7c into phax:master Jan 27, 2022
@shagkur shagkur deleted the error-deprecated branch January 28, 2022 07:05
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 this pull request may close these issues.

2 participants