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

Fix contain check in prefix-value #70

Closed
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
4 participants
@ai
Contributor

ai commented Apr 16, 2013

If I add prefixValue("order"), box-sizing: border-box will be compiled to box-sizing: b-moz-order-box, because Rework try to find any order symbols in CSS value.

This patch fix CSS value check and prefixValue will try to find word, not any letters like order.

This fix need to close bug in Autoprefixer: postcss/autoprefixer#3

@ai ai referenced this pull request Apr 16, 2013

Closed

Prefixed order in b[order]-box #3

@DarthSim

This comment has been minimized.

Show comment
Hide comment
@DarthSim

DarthSim Apr 16, 2013

+1, need this fix

DarthSim commented Apr 16, 2013

+1, need this fix

@tj

View changes

Show outdated Hide outdated lib/plugins/prefix-value.js
@ai

This comment has been minimized.

Show comment
Hide comment
@ai

ai Apr 16, 2013

Contributor

Remove console.log, move function to bottom and add comment.

/\bword\b/ doesn´t work correctly for url(transform.png), because \b pass a lot of symbols.

Contributor

ai commented Apr 16, 2013

Remove console.log, move function to bottom and add comment.

/\bword\b/ doesn´t work correctly for url(transform.png), because \b pass a lot of symbols.

@ai

This comment has been minimized.

Show comment
Hide comment
@ai

ai Apr 16, 2013

Contributor

@visionmedia now everything OK?

Contributor

ai commented Apr 16, 2013

@visionmedia now everything OK?

@tj

This comment has been minimized.

Show comment
Hide comment
@tj

tj Apr 16, 2013

Member

oh right sorry yeah word boundaries don't make sense there

Member

tj commented Apr 16, 2013

oh right sorry yeah word boundaries don't make sense there

@tj

This comment has been minimized.

Show comment
Hide comment
@tj

tj Apr 16, 2013

Member

not sure how i feel about that regexp haha, we'll need a lot better test coverage, values in quotes etc. We might be better off splitting the string on a simple-ish regexp and indexOf() that for the value

Member

tj commented Apr 16, 2013

not sure how i feel about that regexp haha, we'll need a lot better test coverage, values in quotes etc. We might be better off splitting the string on a simple-ish regexp and indexOf() that for the value

@ai

This comment has been minimized.

Show comment
Hide comment
@ai

ai Apr 16, 2013

Contributor

Maybe we could accept this patch, and then we wil try to improve it? Getting Real and same ideas? :)

Contributor

ai commented Apr 16, 2013

Maybe we could accept this patch, and then we wil try to improve it? Getting Real and same ideas? :)

@tj

This comment has been minimized.

Show comment
Hide comment
@tj

tj Apr 17, 2013

Member

no, that's not how I work sorry, I want less things to maintain not more things :p if it's going on master we should at least try and do it correctly

Member

tj commented Apr 17, 2013

no, that's not how I work sorry, I want less things to maintain not more things :p if it's going on master we should at least try and do it correctly

@ai

This comment has been minimized.

Show comment
Hide comment
@ai

ai Apr 23, 2013

Contributor

In Autoprefixer I create own filters, which is 40% faster and hasn't this issue. You can use some ideas from it, when you will try to fix it in Rework.

Contributor

ai commented Apr 23, 2013

In Autoprefixer I create own filters, which is 40% faster and hasn't this issue. You can use some ideas from it, when you will try to fix it in Rework.

@tj

This comment has been minimized.

Show comment
Hide comment
@tj

tj Apr 23, 2013

Member

pushed integrate/prefix-value-fix with an example failing test, we could use a regexp but there's always going to be edge-cases, we just need a quick value parser to properly tokenize strings and url() etc

Member

tj commented Apr 23, 2013

pushed integrate/prefix-value-fix with an example failing test, we could use a regexp but there's always going to be edge-cases, we just need a quick value parser to properly tokenize strings and url() etc

@jonathanong

This comment has been minimized.

Show comment
Hide comment
@jonathanong
Contributor

jonathanong commented Dec 13, 2013

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment