On Fri, 18 Jun 2021 01:23:50 GMT, Michael Strauß <mstra...@openjdk.org> wrote:

> This PR extends data URI support to allow stylesheets to be loaded from data 
> URIs.

I looked at just the public API and spec, and have two comments:

* I don't see any justification for adding 
`Stylesheet::loadBinary(InputStream)` to the public API. See my comments inline.
* Do you intend to support setting a user agent stylesheet via a data URL? The 
docs should be explicit about whether or not a data URI can be used for the 
following methods:
  * 
[Application::setUserAgentStylesheet](https://github.com/openjdk/jfx/blob/master/modules/javafx.graphics/src/main/java/javafx/application/Application.java#L521)
  * 
[Scene::setUserAgentStylesheet](https://github.com/openjdk/jfx/blob/master/modules/javafx.graphics/src/main/java/javafx/scene/Scene.java#L1697)
  * 
[SubScene::setUserAgentStylesheet](https://github.com/openjdk/jfx/blob/master/modules/javafx.graphics/src/main/java/javafx/scene/SubScene.java#L687)

modules/javafx.graphics/src/main/java/javafx/css/Stylesheet.java line 301:

> 299:      * css version or if an I/O error occurs while reading from the 
> stream
> 300:      */
> 301:     public static Stylesheet loadBinary(InputStream stream) throws 
> IOException {

Why do you need to add this public method to the API? I don't see any 
discussion as to why JavaFX applications need it. It looks like it is just 
being used internally by `StyleManager`. Unless there is a compelling reason to 
add this to the API, you will need to make this method package-scope and use an 
accessor to access it from `StyleManager`.

-------------

PR: https://git.openjdk.java.net/jfx/pull/536

Reply via email to