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

Add support for slashes for modern color syntax #82

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

janfaracik
Copy link

This is rather rudimentary so do let me know if a more complete implementation is necessary.

Having an issue when trying to modern color syntax/relative color syntax in Jenkins jenkinsci/jenkins#10078 where the tests are failing due to CSSParser flagging that / isn't supported. This PR adds / so that it no longer fails, added a basic test to confirm too.

The error is below:

Error in expression. (Invalid token "/". Was expecting one of: <S>, <NUMBER>, "inherit", <IDENT>, <STRIN

@rbri
Copy link
Member

rbri commented Dec 19, 2024

Will have a look, maybe we can do it by supporting these colors directly https://developer.mozilla.org/en-US/docs/Web/CSS/color_value/oklch

@rbri
Copy link
Member

rbri commented Dec 22, 2024

@janfaracik ok work has started, i will try to support all the recent color formats. Therefore it might require some days to do that but it will be part of the next release (hopefully still in 2024)

@rbri
Copy link
Member

rbri commented Dec 23, 2024

@janfaracik @timja
have just deployed a new snapshot build 'htmlunitcssparser.version>4.8.0-SNAPSHOT'

Do you have any chance to test this?

@rbri
Copy link
Member

rbri commented Dec 23, 2024

will add relative colors (https://developer.mozilla.org/en-US/docs/Web/CSS/CSS_colors/Relative_colors) after xmas

@timja
Copy link

timja commented Dec 23, 2024

@janfaracik @timja have just deployed a new snapshot build 'htmlunitcssparser.version>4.8.0-SNAPSHOT'

Do you have any chance to test this?

I ran it against the Jan's pull request using:

jenkinsci/jenkins@f3a0705

All passed except for one which I think is a flake, although there is at least one syntax type not being supported as we get this error:

   1.666 [id=453]	WARNING	o.h.DefaultCssErrorHandler#error: CSS error: 'http://localhost:36091/jenkins/static/3349e0f4/jsbundles/styles.css' [1:1571] Error in expression. (Invalid token "/". Was expecting one of: <S>, <NUMBER>, "inherit", <IDENT>, <STRING>, ")", ":", "-", "=", "+", ",", <HASH>, <EMS>, <REM>, <EXS>, <CH>, <VW>, <VH>, <VMIN>, <VMAX>, <LENGTH_PX>, <LENGTH_CM>, <LENGTH_MM>, <LENGTH_IN>, <LENGTH_PT>, <LENGTH_PC>, <LENGTH_Q>, <ANGLE_DEG>, <ANGLE_RAD>, <ANGLE_GRAD>, <ANGLE_TURN>, <TIME_MS>, <TIME_S>, <FREQ_HZ>, <FREQ_KHZ>, <RESOLUTION_DPI>, <RESOLUTION_DPCM>, <PERCENTAGE>, <DIMENSION>, <UNICODE_RANGE>, <URI>, <FUNCTION_CALC>, <FUNCTION_VAR>, <FUNCTION_RGB>, <FUNCTION_HSL>, <FUNCTION>, "progid:".)

https://ci.jenkins.io/job/Core/job/jenkins/job/PR-10078/2/testReport/junit/hudson.model/DirectoryBrowserSupportTest/linux_jdk17___Linux___JDK_17___Build___Test___glob/

@rbri
Copy link
Member

rbri commented Dec 23, 2024

Thanks for the fast testing/feedback. Will inform you after the next update is available.

@rbri
Copy link
Member

rbri commented Dec 24, 2024

@timja do you have a simple chance to extract the http://localhost:36091/jenkins/static/3349e0f4/jsbundles/styles.css file from the jenkis build run and attach it here?

Will be great to use this file for testing and build a regression test out of it.

@timja
Copy link

timja commented Dec 24, 2024

@timja do you have a simple chance to extract the http://localhost:36091/jenkins/static/3349e0f4/jsbundles/styles.css file from the jenkis build run and attach it here?

https://gist.github.com/timja/c63ccf91b14797ec1161322a4fef046c

If you want to reproduce yourself:

gh repo clone jenkinsci/jenkins
cd jenkins
gh pr checkout 10078
mvn clean install -P quick-build

cd test
mvn test -Dtest=hudson.model.DirectoryBrowserSupportTest#glob

The erroring CSS url will be printed in the log and if you're somewhat quick you can grab it, otherwise you can put a j.pause() in the test

@rbri
Copy link
Member

rbri commented Dec 24, 2024

@timja thanks

@rbri
Copy link
Member

rbri commented Jan 8, 2025

@janfaracik @timja
have just deployed a new snapshot build 4.8.0-SNAPSHOT that hopefully likes all the color formats.
Please give it a try.

@janfaracik
Copy link
Author

@janfaracik @timja have just deployed a new snapshot build 4.8.0-SNAPSHOT that hopefully likes all the color formats. Please give it a try.

Fantastic, thanks! Running now - jenkinsci/jenkins#10078

@timja
Copy link

timja commented Jan 8, 2025

Thanks worked great, looks like my previous attempt didn't work setting a dependency management, I forgot the dependency was shaded, this worked instead: jenkinsci/jenkins-test-harness-htmlunit#173

With this diff:

diff --git a/test/pom.xml b/test/pom.xml
index 5f8d44d580..2760314326 100644
--- a/test/pom.xml
+++ b/test/pom.xml
@@ -138,9 +138,9 @@ THE SOFTWARE.
       </dependency>

       <dependency>
-        <groupId>org.htmlunit</groupId>
-        <artifactId>htmlunit</artifactId>
-        <version>4.8.0-SNAPSHOT</version>
+        <groupId>org.jenkins-ci.main</groupId>
+        <artifactId>jenkins-test-harness-htmlunit</artifactId>
+        <version>199.ved3a_ecd38a_5d</version>
       </dependency>
     </dependencies>
   </dependencyManagement>

Test output:

[INFO] Running hudson.model.DirectoryBrowserSupportTest
=== Starting glob(hudson.model.DirectoryBrowserSupportTest)
   0.057 [id=20]	INFO	o.jvnet.hudson.test.WarExploder#explode: Using jenkins.war resources from /Users/timja/code/jenkins/jenkins/war/target/jenkins
   0.331 [id=20]	INFO	o.jvnet.hudson.test.JenkinsRule#createWebServer2: Running on http://localhost:49507/jenkins/
   0.450 [id=20]	INFO	jenkins.model.Jenkins#<init>: Starting version 2.493-SNAPSHOT
   0.505 [id=35]	INFO	jenkins.InitReactorRunner$1#onAttained: Started initialization
   1.181 [id=36]	INFO	hudson.PluginManager#considerDetachedPlugin: Loading a detached plugin /var/folders/7p/92qym3cj4p9bzr330tcx2m9w0000gn/T/jenkins12234640861398740781/command-launcher.jpi as a dependency of gradle
   1.192 [id=36]	INFO	hudson.PluginManager#considerDetachedPlugin: Loading a detached plugin /var/folders/7p/92qym3cj4p9bzr330tcx2m9w0000gn/T/jenkins12234640861398740781/apache-httpcomponents-client-4-api.jpi as a dependency of gradle
   1.223 [id=36]	INFO	hudson.PluginManager#considerDetachedPlugin: Loading a detached plugin /var/folders/7p/92qym3cj4p9bzr330tcx2m9w0000gn/T/jenkins12234640861398740781/jdk-tool.jpi as a dependency of gradle
   1.236 [id=36]	INFO	hudson.PluginManager#considerDetachedPlugin: Loading a detached plugin /var/folders/7p/92qym3cj4p9bzr330tcx2m9w0000gn/T/jenkins12234640861398740781/trilead-api.jpi as a dependency of gradle
   1.266 [id=36]	INFO	hudson.PluginManager#considerDetachedPlugin: Loading a detached plugin /var/folders/7p/92qym3cj4p9bzr330tcx2m9w0000gn/T/jenkins12234640861398740781/eddsa-api.jpi as a dependency of gradle
   1.270 [id=36]	INFO	hudson.PluginManager#considerDetachedPlugin: Loading a detached plugin /var/folders/7p/92qym3cj4p9bzr330tcx2m9w0000gn/T/jenkins12234640861398740781/gson-api.jpi as a dependency of gradle
   1.285 [id=36]	INFO	hudson.PluginManager#considerDetachedPlugin: Loading a detached plugin /var/folders/7p/92qym3cj4p9bzr330tcx2m9w0000gn/T/jenkins12234640861398740781/mina-sshd-api-core.jpi as a dependency of gradle
   1.304 [id=36]	INFO	hudson.PluginManager#considerDetachedPlugin: Loading a detached plugin /var/folders/7p/92qym3cj4p9bzr330tcx2m9w0000gn/T/jenkins12234640861398740781/mina-sshd-api-common.jpi as a dependency of gradle
   1.315 [id=36]	INFO	hudson.PluginManager#considerDetachedPlugin: Loading a detached plugin /var/folders/7p/92qym3cj4p9bzr330tcx2m9w0000gn/T/jenkins12234640861398740781/sshd.jpi as a dependency of gradle
   1.513 [id=45]	INFO	jenkins.InitReactorRunner$1#onAttained: Listed all plugins
   1.527 [id=41]	INFO	j.b.api.BouncyCastlePlugin#start: /Users/timja/code/jenkins/jenkins/test/target/j h5036321073316107752/plugins/bouncycastle-api/WEB-INF/optional-lib not found; for non RealJenkinsRule this is fine and can be ignored.
   2.127 [id=45]	INFO	jenkins.InitReactorRunner$1#onAttained: Prepared all plugins
   2.133 [id=45]	INFO	jenkins.InitReactorRunner$1#onAttained: Started all plugins
   2.133 [id=46]	INFO	jenkins.InitReactorRunner$1#onAttained: Augmented all extensions
   2.525 [id=34]	INFO	jenkins.InitReactorRunner$1#onAttained: System config loaded
   2.526 [id=35]	INFO	jenkins.InitReactorRunner$1#onAttained: System config adapted
   2.526 [id=33]	INFO	jenkins.InitReactorRunner$1#onAttained: Loaded all jobs
   2.529 [id=33]	INFO	jenkins.InitReactorRunner$1#onAttained: Configuration for all jobs updated
   2.594 [id=38]	INFO	jenkins.InitReactorRunner$1#onAttained: Completed initialization
   2.673 [id=20]	WARNING	o.j.r.u.AnonymousClassWarnings#warn: Attempt to (de-)serialize anonymous class hudson.model.DirectoryBrowserSupportTest$2 in file:/Users/timja/code/jenkins/jenkins/test/target/test-classes/; see: https://jenkins.io/redirect/serialization-of-anonymous-classes/
  24.851 [id=20]	INFO	hudson.lifecycle.Lifecycle#onStatusUpdate: Stopping Jenkins
WARN: The method class org.apache.commons.logging.impl.SLF4JLogFactory#release() was invoked.
WARN: Please see http://www.slf4j.org/codes.html#release for an explanation.
  24.871 [id=20]	INFO	hudson.lifecycle.Lifecycle#onStatusUpdate: Jenkins stopped
  24.899 [id=20]	INFO	o.j.h.t.TemporaryDirectoryAllocator#dispose: deleting /Users/timja/code/jenkins/jenkins/test/target/j h5036321073316107752

@rbri
Copy link
Member

rbri commented Jan 8, 2025

Ok, great, will try to find some time to make a full HtmlUnit release this weekend

@rbri
Copy link
Member

rbri commented Jan 9, 2025

@janfaracik

Fantastic, thanks! Running now - jenkinsci/jenkins#10078

Failed - something for me or because of other reasons?

@timja
Copy link

timja commented Jan 9, 2025

@janfaracik

Fantastic, thanks! Running now - jenkinsci/jenkins#10078

Failed - something for me or because of other reasons?

It’s not actually using the snapshot. Tests are unfortunately a bit flakey atm

@rbri
Copy link
Member

rbri commented Jan 9, 2025

@janfaracik @timja writing more unit tests always leads to more fixes, have just deployed another new snapshot build 4.8.0-SNAPSHOT

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.

3 participants