Re: RFR: 8267551: Support loading images from inline data-URIs [v14]
On Sat, 5 Jun 2021 15:42:22 GMT, Michael Strauß wrote:
>> This PR adds support for loading images from [inline data
>> URIs](https://en.wikipedia.org/wiki/Data_URI_scheme), which is also widely
>> supported by web browsers. This enables developers to package small images
>> in CSS files, rather than separately deploying the images alongside the CSS
>> file.
>
> Michael Strauß has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Addressed review comments
The docs updates look good, with a few minor comments.
modules/javafx.graphics/src/main/java/javafx/scene/image/Image.java line 68:
> 66: * images from a specified URL.
> 67: *
> 68: * Supported image formats are:
Can you revert this formatting change to avoid unnecessary diffs? (I mean the
one that moved the sentence up to be on the same line as the ``. The removal
of the unneeded `` is fine)
The reason for this is to avoid extra diffs in public API. It will make it
easier to see what needs to go into the CSR.
modules/javafx.graphics/src/main/java/javafx/scene/image/Image.java line 76:
> 74: *
> 75: *
> 76: * Images can be resized as they are loaded (for example to reduce the
> amount of
Revert
modules/javafx.graphics/src/main/java/javafx/scene/image/Image.java line 619:
> 617: * Constructs an {@code Image} with content loaded from the
> specified URL.
> 618: *
> 619: * @param url a resource path, file path or URL
Please add a comma after `file path` (we prefer the Oxford comma in our docs).
modules/javafx.graphics/src/main/java/javafx/scene/image/Image.java line 632:
> 630: * using the specified parameters.
> 631: *
> 632: * @param url a resource path, file path or URL
Comma after `file path`
modules/javafx.graphics/src/main/java/javafx/scene/image/Image.java line 647:
> 645: * using the specified parameters.
> 646: *
> 647: * @param url a resource path, file path or URL
Comma after `file path`
-
PR: https://git.openjdk.java.net/jfx/pull/508
Re: RFR: 8267551: Support loading images from inline data-URIs [v14]
On Sat, 5 Jun 2021 15:42:22 GMT, Michael Strauß wrote: >> This PR adds support for loading images from [inline data >> URIs](https://en.wikipedia.org/wiki/Data_URI_scheme), which is also widely >> supported by web browsers. This enables developers to package small images >> in CSS files, rather than separately deploying the images alongside the CSS >> file. > > Michael Strauß has updated the pull request incrementally with one additional > commit since the last revision: > > Addressed review comments Yes, it is a good question as to whether we should add Data URI support in the core of JavaFX or provide it via some other means. I agree that we wouldn't want to implement it in JavaFX with a pluggable protocol handler, so if we go that route we will end up pushing it off to the app. The other alternative that was mentioned (as a comment in the JBS bug) was to push for direct support in the JDK. Since that was requested some years ago and rejected -- see [JDK-4188739](https://bugs.openjdk.java.net/browse/JDK-4188739) -- I doubt that is likely. My gut feel is that adding Data URI support to JavaFX is a worthwhile feature, but it might warrant further discussion. - PR: https://git.openjdk.java.net/jfx/pull/508
Re: RFR: 8267551: Support loading images from inline data-URIs [v14]
> This PR adds support for loading images from [inline data > URIs](https://en.wikipedia.org/wiki/Data_URI_scheme), which is also widely > supported by web browsers. This enables developers to package small images in > CSS files, rather than separately deploying the images alongside the CSS file. Michael Strauß has updated the pull request incrementally with one additional commit since the last revision: Addressed review comments - Changes: - all: https://git.openjdk.java.net/jfx/pull/508/files - new: https://git.openjdk.java.net/jfx/pull/508/files/315523c5..f6201b7e Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jfx=508=13 - incr: https://webrevs.openjdk.java.net/?repo=jfx=508=12-13 Stats: 107 lines in 2 files changed: 29 ins; 57 del; 21 mod Patch: https://git.openjdk.java.net/jfx/pull/508.diff Fetch: git fetch https://git.openjdk.java.net/jfx pull/508/head:pull/508 PR: https://git.openjdk.java.net/jfx/pull/508
