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

Modifications to support line numbers within CSS #79

Closed
sidkshatriya opened this issue May 31, 2016 · 5 comments
Closed

Modifications to support line numbers within CSS #79

sidkshatriya opened this issue May 31, 2016 · 5 comments
Assignees
Labels

Comments

@sidkshatriya
Copy link
Contributor

sidkshatriya commented May 31, 2016

As discussed in #78 (comment)

The canonical validator points out issues in the CSS on the exact line of the HTML document. In our case the line number we provide is the line number of the containing <style> tag. To provide exact line number within the CSS we might need to make changes to the the PHP parser library ( https://packagist.org/packages/sabberworm/php-css-parser ).

So we need to make two changes:

  • Make changes to our fork ( https://github.com/Lullabot/PHP-CSS-Parser) of sabberworm/php-css-parser to support line numbers. (This could later be submitted as a pull request upstream; but composer allows us to point to our repo if we need to also)
  • Make changes to our amp-library code to support validation error line numbers within the CSS
@sidkshatriya
Copy link
Contributor Author

sidkshatriya commented May 31, 2016

You may track the sabberworm/php-css-parser related changes in https://github.com/Lullabot/PHP-CSS-Parser/tree/line-number-support-in-css-objects

PR #80 should be run only in conjunction with the above branch.

@sidkshatriya
Copy link
Contributor Author

Added a pull request to the sabberworm/php-css-parser project MyIntervals/PHP-CSS-Parser#105

@sidkshatriya
Copy link
Contributor Author

Made a huge amount of progress on MyIntervals/PHP-CSS-Parser#105

See the PR for changes. Hopefully it should not take too long to get this change accepted into mainline

@sidkshatriya
Copy link
Contributor Author

The sabberworm PR was merged to master

@sidkshatriya
Copy link
Contributor Author

With:

This ticket can be now closed.

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

No branches or pull requests

1 participant