Not sure how it happened, but there seems to be a css syntax error hiding in plain sight.

Remaining tasks

Determine if csslint can be incorporated into automated testing. ( not for style issue -- just syntax errors. )

Related project

https://www.drupal.org/project/drupalci

CommentFileSizeAuthor
#5 csslint.zip840 bytesattiks
plain-sight-0.patch437 bytesmartin107

Comments

martin107’s picture

martin107 commented
Related issues: +#2296261: Misalligned Type field in Drupal 8.x views

If you look at the last patch from this issue you can see it creep in

looks like it has been hiding since March 31, 2015

so it can't be a big issue :)

martin107’s picture

martin107 commented

Over in #2472269: Fix syntax errors in Drupal\Component\Plugin's composer.json, Add test

when a syntax bug was discovered in a json file the response was

An integration test - load all the composer.json files report any syntax error at the unit testing stage

The next natural question is can we do this for css - as usual stackoverflow has asked the question already

http://stackoverflow.com/questions/3330571/how-can-i-validate-css-within...

If someone can point me a css syntax checking tool ( from within php ) then I will write the test.... how is that!

martin107’s picture

martin107 commented
Component: views_ui.module » CSS

changing component to css

attiks’s picture

attiks commented

I ran grunt csslint on all css files, output added below, limited to core css

Running "csslint:strict" (csslint) task
Linting core/modules/locale/css/locale.admin.css...ERROR
[L71:C1]
WARNING: Rule is empty. Rules without any properties specified should be removed. (empty-rules) Browsers: All
Linting core/modules/system/css/system.theme.css...ERROR
[L328:C1]
>> ERROR: Unknown @ rule: @-ms-keyframes. This rule looks for recoverable syntax errors. (errors) Browsers: All
Linting core/modules/views_ui/css/views_ui.admin.theme.css...ERROR
[L845:C1]
>> ERROR: Unexpected token '}' at line 845, col 1. This rule looks for recoverable syntax errors. (errors) Browsers: All
Linting core/tests/Drupal/Tests/Core/Asset/css_test_files/css_input_with_import.css.optimized.css...ERROR
[L3:C1]
>> ERROR: @import not allowed here. This rule looks for recoverable syntax errors. (errors) Browsers: All
[L3:C45]
>> ERROR: @import not allowed here. This rule looks for recoverable syntax errors. (errors) Browsers: All
Linting core/tests/Drupal/Tests/Core/Asset/css_test_files/css_input_without_import.css...ERROR
[L57:C19]
>> ERROR: Expected LBRACE at line 57, col 19. This rule looks for recoverable syntax errors. (errors) Browsers: All
[L57:C19]
>> ERROR: Unexpected token ';' at line 57, col 19. This rule looks for recoverable syntax errors. (errors) Browsers: All
[L58:C14]
>> ERROR: Expected LBRACE at line 58, col 14. This rule looks for recoverable syntax errors. (errors) Browsers: All
[L58:C14]
>> ERROR: Unexpected token ';' at line 58, col 14. This rule looks for recoverable syntax errors. (errors) Browsers: All
[L59:C1]
>> ERROR: Unexpected token '}' at line 59, col 1. This rule looks for recoverable syntax errors. (errors) Browsers: All
Linting core/tests/Drupal/Tests/Core/Asset/css_test_files/css_input_without_import.css.optimized.css...ERROR
[L4:C319]
>> ERROR: Expected LBRACE at line 4, col 319. This rule looks for recoverable syntax errors. (errors) Browsers: All
[L4:C323]
>> ERROR: Expected LBRACE at line 4, col 323. This rule looks for recoverable syntax errors. (errors) Browsers: All
[L4:C323]
>> ERROR: Unexpected token ';' at line 4, col 323. This rule looks for recoverable syntax errors. (errors) Browsers: All
[L4:C330]
>> ERROR: Expected LBRACE at line 4, col 330. This rule looks for recoverable syntax errors. (errors) Browsers: All
[L4:C334]
>> ERROR: Expected LBRACE at line 4, col 334. This rule looks for recoverable syntax errors. (errors) Browsers: All
[L4:C334]
>> ERROR: Unexpected token ';' at line 4, col 334. This rule looks for recoverable syntax errors. (errors) Browsers: All
[L4:C335]
>> ERROR: Unexpected token '}' at line 4, col 335. This rule looks for recoverable syntax errors. (errors) Browsers: All
Linting core/themes/seven/css/components/form.css...ERROR
[L14:C1]
>> ERROR: Unknown @ rule: @-moz-document. This rule looks for recoverable syntax errors. (errors) Browsers: All
Linting core/vendor/phpunit/php-code-coverage/src/CodeCoverage/Report/HTML/Renderer/Template/css/style.css...ERROR
[L55:C1]
WARNING: Rule is empty. Rules without any properties specified should be removed. (empty-rules) Browsers: All
attiks’s picture

attiks commented
StatusFileSize
new csslint.zip840 bytes

If you want to run this as well you need the following:

Run npm install grunt-contrib-csslint --save-dev in the root directory

Gruntfile.js and .csslintrc in attached zip, extract inside the root directory

Run grunt csslint to test

attiks’s picture

attiks commented
Status: Needs review » Closed (duplicate)

This is already covered in #2408525: Rewrite Views UI CSS inline with our CSS standards - Part 1

For testing there's https://github.com/lewisnyman/drupalcore-frontend-toolkit

attiks’s picture

attiks commented

This is already covered in #2408525: Rewrite Views UI CSS inline with our CSS standards - Part 1

For testing there's https://github.com/lewisnyman/drupalcore-frontend-toolkit

martin107’s picture

martin107 commented
Status: Closed (duplicate) » Needs work
Issue tags: -Quick fix

Thanks @attiks

I run something similar ... on my own work - Mostly as it bites me in the bum every day!

I am reopening this as there are 2 things that #2408525: Rewrite Views UI CSS inline with our CSS standards - Part 1 does not cover.

1) The syntax bugs identified here are not coding standard refinements - and need fixing in their own right.
2) The real advantage of a phpunit integration test is that, front-end or backend, it stops ALL developers, myself included, from submitting corrupted files.

I will try and find sometime this weekend.

attiks’s picture

attiks commented

True, good reasons to reopen, I'll ping Lewis on Twitter, he might have ideas for testing.

lewisnyman’s picture

lewisnyman
he/him
commented
Issue tags: +frontend, +CSS

Ok that sounds really good! If we could catch these major errors automatically that would be good.

martin107’s picture

martin107 commented
Title: view_ui.admin.theme.css syntax error. » Prevent css syntax errors from ever getting the RTBC
Category: Bug report » Task
Issue summary: View changes

To answer my own question from #2 - I have had more time to search for a solution

There is a php library,

https://github.com/sabberworm/PHP-CSS-Parser

When I run through a standard checklist asking is is safe to pull in the library ... well the responses are not great

1) Are there recent maintenance updates ..evidence for widespread support.

Err not really.

2) What are the terms of the license

PHP-CSS-Parser is freely distributable under the terms of an MIT-style license.

Copyright (c) 2011 Raphael Schweikert, http://sabberworm.com/

I'm not sure if this would conflict with any of Drupal's in-take conditions - can someone else speak to that?

3) does it work ?

It has the potential to meet our needs

$oCssParser = new Sabberworm\CSS\Parser(file_get_contents('somefile.css'), Sabberworm\CSS\Settings::create()->beStrict());

can be used to reject files with syntax errors.

Changing the subject completely

can I take the legs away from one of my arguments #8.1

1) The syntax bugs identified here are not coding standard refinements - and need fixing in their own right.

After reading the related issue I have confidence the syntax error will be dealt with elsewhere in a more community orientated way. That is I plan to help out in the appropriate issue.

So barring the location of a well supported php library - I find myself looking for other ways to block css files with syntax error from ever being RTBC'ed.

I think there is a initiative to improve testbot -- by using jenkins/travis instead of the current testbot.... I need to find out more...

but if that is the case then csslint could be run as part of the review process... it would be annoying to have more than one arbiter of syntactically correct css .. as it is just going to lead to a bikeshed moment when the two standards disagree.

martin107’s picture

martin107 commented
Title: Prevent css syntax errors from ever getting the RTBC » Prevent css syntax errors from ever getting to RTBC
martin107’s picture

martin107 commented
Issue summary: View changes

adding https://www.drupal.org/project/drupalci to the issue summary

Version: 8.0.x-dev » 8.1.x-dev

Drupal 8.0.6 was released on April 6 and is the final bugfix release for the Drupal 8.0.x series. Drupal 8.0.x will not receive any further development aside from security fixes. Drupal 8.1.0-rc1 is now available and sites should prepare to update to 8.1.0.

Bug reports should be targeted against the 8.1.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.2.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.1.x-dev » 8.2.x-dev

Drupal 8.1.9 was released on September 7 and is the final bugfix release for the Drupal 8.1.x series. Drupal 8.1.x will not receive any further development aside from security fixes. Drupal 8.2.0-rc1 is now available and sites should prepare to upgrade to 8.2.0.

Bug reports should be targeted against the 8.2.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.3.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.2.x-dev » 8.3.x-dev

Drupal 8.2.6 was released on February 1, 2017 and is the final full bugfix release for the Drupal 8.2.x series. Drupal 8.2.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.3.0 on April 5, 2017. (Drupal 8.3.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.3.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.4.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.3.x-dev » 8.4.x-dev

Drupal 8.3.6 was released on August 2, 2017 and is the final full bugfix release for the Drupal 8.3.x series. Drupal 8.3.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.4.0 on October 4, 2017. (Drupal 8.4.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.4.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.5.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

martin107’s picture

martin107 commented
Status: Needs work » Closed (outdated)