Re: RFR: 8267551: Support loading images from inline data-URIs [v19]
On Mon, 14 Jun 2021 08:50:25 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: > > changes Looks good to me too. - Marked as reviewed by arapte (Reviewer). PR: https://git.openjdk.java.net/jfx/pull/508
Re: RFR: 8267551: Support loading images from inline data-URIs [v19]
On Mon, 14 Jun 2021 08:50:25 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: > > changes Looks good. I reviewed the CSR, so you can move that to Finalized now. - Marked as reviewed by kcr (Lead). PR: https://git.openjdk.java.net/jfx/pull/508
Re: RFR: 8267551: Support loading images from inline data-URIs [v19]
On Mon, 14 Jun 2021 08:50:25 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: > > changes Made changes as per review. - PR: https://git.openjdk.java.net/jfx/pull/508
Re: RFR: 8267551: Support loading images from inline data-URIs [v19]
> 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: changes - Changes: - all: https://git.openjdk.java.net/jfx/pull/508/files - new: https://git.openjdk.java.net/jfx/pull/508/files/5cbfa701..8023fb81 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jfx=508=18 - incr: https://webrevs.openjdk.java.net/?repo=jfx=508=17-18 Stats: 17 lines in 1 file changed: 7 ins; 5 del; 5 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
Re: RFR: 8267551: Support loading images from inline data-URIs [v18]
> 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: changes as per review - Changes: - all: https://git.openjdk.java.net/jfx/pull/508/files - new: https://git.openjdk.java.net/jfx/pull/508/files/ed498ef6..5cbfa701 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jfx=508=17 - incr: https://webrevs.openjdk.java.net/?repo=jfx=508=16-17 Stats: 23 lines in 2 files changed: 13 ins; 8 del; 2 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
Re: RFR: 8267551: Support loading images from inline data-URIs [v17]
On Wed, 9 Jun 2021 14:20:41 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:
>
> doc
I reviewed and tested the implementation. I left a couple questions inline.
modules/javafx.graphics/src/main/java/com/sun/javafx/iio/ImageStorage.java line
314:
> 312: }
> 313:
> 314: theStream = new
> ByteArrayInputStream(dataUri.getData());
By parsing the Data URI before calling `ImageTools.createInputStream` our
internal handling will take precedence over a custom handler that an
application might have installed. I'm not sure this is what we want.
modules/javafx.graphics/src/main/java/com/sun/javafx/util/DataURI.java line 185:
> 183: @Override
> 184: public String toString() {
> 185: if (originalData.length() < 30) {
I think this should be `< 32`. As it stands, if length is `30` or `31` the data
portion of the string will be 31 characters with either 2 or 3 middle chars
replaced by `"..."`. In those cases it would be better to just use the original
data.
-
PR: https://git.openjdk.java.net/jfx/pull/508
Re: RFR: 8267551: Support loading images from inline data-URIs [v17]
On Wed, 9 Jun 2021 14:20:41 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: > > doc I've updated the CSR with the documentation diffs. - PR: https://git.openjdk.java.net/jfx/pull/508
Re: RFR: 8267551: Support loading images from inline data-URIs [v16]
On Wed, 9 Jun 2021 13:18:29 GMT, Kevin Rushforth wrote: >> Michael Strauß has updated the pull request incrementally with one >> additional commit since the last revision: >> >> added comment, simplified DataURI.matchScheme() > > modules/javafx.graphics/src/main/java/javafx/scene/image/Image.java line 95: > >> 93: * the protocol handlers that are registered for the application. >> 94: * >> 95: * If a URL uses the "data" scheme, the data must be base64-encoded > > I think these two sentences should be in the same paragraph. Ok. - PR: https://git.openjdk.java.net/jfx/pull/508
Re: RFR: 8267551: Support loading images from inline data-URIs [v17]
> 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: doc - Changes: - all: https://git.openjdk.java.net/jfx/pull/508/files - new: https://git.openjdk.java.net/jfx/pull/508/files/e490cc04..ed498ef6 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jfx=508=16 - incr: https://webrevs.openjdk.java.net/?repo=jfx=508=15-16 Stats: 2 lines in 1 file changed: 0 ins; 1 del; 1 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
Re: RFR: 8267551: Support loading images from inline data-URIs [v16]
On Sat, 5 Jun 2021 17:31:17 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: > > added comment, simplified DataURI.matchScheme() The docs look good. I left one minor comment inline. You can update the CSR when ready. As I mentioned in a comment in the CSR, the `Specification` section should consist of the diffs to the API documentation. Make sure you note what API element (e.g., class doc, `Image(String url)` constructor, and so forth) each change is for, if it isn't clear from the diffs. You can omit the removal of the `` html tags and the change from `example.` to `example:` from the diffs in the CSR. modules/javafx.graphics/src/main/java/javafx/scene/image/Image.java line 95: > 93: * the protocol handlers that are registered for the application. > 94: * > 95: * If a URL uses the "data" scheme, the data must be base64-encoded I think these two sentences should be in the same paragraph. - PR: https://git.openjdk.java.net/jfx/pull/508
Re: RFR: 8267551: Support loading images from inline data-URIs [v13]
On Sat, 5 Jun 2021 16:12:36 GMT, Kevin Rushforth wrote: >> `resource` is never `null` at this point, and it is already trimmed. Also, >> contrary to the comment, I cannot see this method being used anywhere in >> tests, so I made it private. > > If that is guaranteed by all callers of this method (which is easier to check > now that you made it private), this is fine. This method is only used once, and only after `resource` has already been dereferenced. So if it was indeed `null`, we would have errored out with `NullPointerException` before calling this method. - PR: https://git.openjdk.java.net/jfx/pull/508
Re: RFR: 8267551: Support loading images from inline data-URIs [v16]
On Sat, 5 Jun 2021 17:31:17 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: > > added comment, simplified DataURI.matchScheme() Since this implementation doesn't surface any new API, it will not prevent us from removing it at any future time when the "data" protocol might be supported by the JDK. - PR: https://git.openjdk.java.net/jfx/pull/508
Re: RFR: 8267551: Support loading images from inline data-URIs [v13]
On Fri, 4 Jun 2021 22:00:03 GMT, Kevin Rushforth wrote:
>> Michael Strauß has updated the pull request with a new target base due to a
>> merge or a rebase. The pull request now contains 15 commits:
>>
>> - Merge branch 'master' into feature/image-datauri
>>
>># Conflicts:
>>#
>> modules/javafx.graphics/src/test/java/test/com/sun/javafx/iio/ImageStorageTest.java
>> - Accept regular file paths, change javadoc
>> - Merge branch 'openjdk:master' into feature/image-datauri
>> - Rename DataURI.isValidURI
>> - Reverted a change
>> - Allow leading and trailing whitespace in data URI
>> - Removed test
>> - Changed data URI detection
>> - Merge branch 'master' into feature/image-datauri
>> - Moved test
>> - ... and 5 more:
>> https://git.openjdk.java.net/jfx/compare/7b7050c4...315523c5
>
> modules/javafx.graphics/src/main/java/com/sun/javafx/util/DataURI.java line
> 76:
>
>> 74:
>> 75: if (Character.isWhitespace(uri.charAt(0))) {
>> 76: uri = uri.trim();
>
> Same question as above.
Removed the conditional branch.
-
PR: https://git.openjdk.java.net/jfx/pull/508
Re: RFR: 8267551: Support loading images from inline data-URIs [v13]
On Sat, 5 Jun 2021 15:52:01 GMT, Kevin Rushforth wrote:
>> My idea was to avoid repeatedly allocating a new String with the data
>> contained in the URI. Since `matchScheme` is called at least twice (maybe
>> more), that would be a total of at least three copies of the URI data until
>> we parse it. Granted, this would only apply for URIs that contain leading or
>> trailing whitespace (since `trim()` is specified to return the same String
>> instance if it contains no leading or trailing whitespace).
>
> I see. How about something like this then?
>
>
> if (uri == null || uri.isEmpty()) {
> return false;
> }
>
> if (Character.isWhiteSpace(uri.charAt(0)) {
> uri = uri.trim();
> }
>
> if (uri.length() < 6) {
> return false;
> }
>
>
> The rest of the method can then work on a trimmed string.
I replaced it with another implementation that is effectively the same as this.
-
PR: https://git.openjdk.java.net/jfx/pull/508
Re: RFR: 8267551: Support loading images from inline data-URIs [v16]
> 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: added comment, simplified DataURI.matchScheme() - Changes: - all: https://git.openjdk.java.net/jfx/pull/508/files - new: https://git.openjdk.java.net/jfx/pull/508/files/29178487..e490cc04 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jfx=508=15 - incr: https://webrevs.openjdk.java.net/?repo=jfx=508=14-15 Stats: 20 lines in 2 files changed: 3 ins; 14 del; 3 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
Re: RFR: 8267551: Support loading images from inline data-URIs [v13]
On Sat, 5 Jun 2021 17:10:29 GMT, John Hendrikx wrote: >> Maybe add a brief comment to that effect? > > Ok clear thanks I added a comment that explains that. - PR: https://git.openjdk.java.net/jfx/pull/508
Re: RFR: 8267551: Support loading images from inline data-URIs [v13]
On Sat, 5 Jun 2021 16:10:59 GMT, Kevin Rushforth wrote: >> In this specific case, image loading has failed for some reason. The call to >> `DataURI.tryParse` is only there to potentially call `DataURI.toString()` >> for a truncated log output, instead of logging the entire `url` String. If >> data-URI truncation is not wanted, this code can be reverted. > > Maybe add a brief comment to that effect? Ok clear thanks - PR: https://git.openjdk.java.net/jfx/pull/508
Re: RFR: 8267551: Support loading images from inline data-URIs [v15]
> 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: javadoc fixes - Changes: - all: https://git.openjdk.java.net/jfx/pull/508/files - new: https://git.openjdk.java.net/jfx/pull/508/files/f6201b7e..29178487 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jfx=508=14 - incr: https://webrevs.openjdk.java.net/?repo=jfx=508=13-14 Stats: 8 lines in 1 file changed: 2 ins; 0 del; 6 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
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 [v13]
On Sat, 5 Jun 2021 15:44:38 GMT, Michael Strauß wrote:
>> modules/javafx.graphics/src/main/java/javafx/css/converter/URLConverter.java
>> line 98:
>>
>>> 96: }
>>> 97:
>>> 98: private URL resolve(String stylesheetUrl, String resource) {
>>
>> Why was this change done? It seems unnecessary and possibly unwanted.
>
> `resource` is never `null` at this point, and it is already trimmed. Also,
> contrary to the comment, I cannot see this method being used anywhere in
> tests, so I made it private.
If that is guaranteed by all callers of this method (which is easier to check
now that you made it private), this is fine.
-
PR: https://git.openjdk.java.net/jfx/pull/508
Re: RFR: 8267551: Support loading images from inline data-URIs [v13]
On Sat, 5 Jun 2021 15:49:16 GMT, Michael Strauß wrote:
>> modules/javafx.graphics/src/main/java/com/sun/javafx/css/StyleManager.java
>> line 776:
>>
>>> 774: } else {
>>> 775: logger.warning("Error loading
>>> image: " + url);
>>> 776: }
>>
>> So if I understand this correctly, an Image is first loaded via the standard
>> mechanism, and if that fails for any reason (`image.isError()`) a second
>> attempt is made through `DataURI` ?
>>
>> Would it not be better to register a new protocol so `new Image` would just
>> succeed for data uri's ?
>
> In this specific case, image loading has failed for some reason. The call to
> `DataURI.tryParse` is only there to potentially call `DataURI.toString()` for
> a truncated log output, instead of logging the entire `url` String. If
> data-URI truncation is not wanted, this code can be reverted.
Maybe add a brief comment to that effect?
-
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 [v13]
On Sat, 5 Jun 2021 15:08:29 GMT, Michael Strauß wrote: >> modules/javafx.graphics/src/main/java/com/sun/javafx/util/DataURI.java line >> 185: >> >>> 183: >>> 184: return originalUri.substring(0, originalUri.length() - >>> originalData.length()) >>> 185: + originalData.substring(0, 14) + "..." + >>> originalData.substring(originalData.length() - 14); >> >> Why truncate it? > > The idea was to not clutter logs with large pieces of data. Do you think that > the entire data string should be logged? Your explanation is fine. And since this is an internal class, it doesn't matter all that much. - PR: https://git.openjdk.java.net/jfx/pull/508
Re: RFR: 8267551: Support loading images from inline data-URIs [v13]
On Sat, 5 Jun 2021 15:17:09 GMT, Michael Strauß wrote:
>> In that case, it might be clearer and simpler to just call `trim()` on the
>> input String before doing anything with it, unless there is a reason not to.
>
> My idea was to avoid repeatedly allocating a new String with the data
> contained in the URI. Since `matchScheme` is called at least twice (maybe
> more), that would be a total of at least three copies of the URI data until
> we parse it. Granted, this would only apply for URIs that contain leading or
> trailing whitespace (since `trim()` is specified to return the same String
> instance if it contains no leading or trailing whitespace).
I see. How about something like this then?
if (uri == null || uri.isEmpty()) {
return false;
}
if (Character.isWhiteSpace(uri.charAt(0)) {
uri = uri.trim();
}
if (uri.length() < 6) {
return false;
}
The rest of the method can then work on a trimmed string.
-
PR: https://git.openjdk.java.net/jfx/pull/508
Re: RFR: 8267551: Support loading images from inline data-URIs [v13]
On Sat, 5 Jun 2021 08:21:14 GMT, John Hendrikx wrote:
>> Michael Strauß has updated the pull request with a new target base due to a
>> merge or a rebase. The pull request now contains 15 commits:
>>
>> - Merge branch 'master' into feature/image-datauri
>>
>># Conflicts:
>>#
>> modules/javafx.graphics/src/test/java/test/com/sun/javafx/iio/ImageStorageTest.java
>> - Accept regular file paths, change javadoc
>> - Merge branch 'openjdk:master' into feature/image-datauri
>> - Rename DataURI.isValidURI
>> - Reverted a change
>> - Allow leading and trailing whitespace in data URI
>> - Removed test
>> - Changed data URI detection
>> - Merge branch 'master' into feature/image-datauri
>> - Moved test
>> - ... and 5 more:
>> https://git.openjdk.java.net/jfx/compare/7b7050c4...315523c5
>
> modules/javafx.graphics/src/main/java/com/sun/javafx/css/StyleManager.java
> line 776:
>
>> 774: } else {
>> 775: logger.warning("Error loading
>> image: " + url);
>> 776: }
>
> So if I understand this correctly, an Image is first loaded via the standard
> mechanism, and if that fails for any reason (`image.isError()`) a second
> attempt is made through `DataURI` ?
>
> Would it not be better to register a new protocol so `new Image` would just
> succeed for data uri's ?
In this specific case, image loading has failed for some reason. The call to
`DataURI.tryParse` is only there to potentially call `DataURI.toString()` for a
truncated log output, instead of logging the entire `url` String. If data-URI
truncation is not wanted, this code can be reverted.
-
PR: https://git.openjdk.java.net/jfx/pull/508
Re: RFR: 8267551: Support loading images from inline data-URIs [v13]
On Fri, 4 Jun 2021 22:09:09 GMT, Kevin Rushforth wrote:
>> Michael Strauß has updated the pull request with a new target base due to a
>> merge or a rebase. The pull request now contains 15 commits:
>>
>> - Merge branch 'master' into feature/image-datauri
>>
>># Conflicts:
>>#
>> modules/javafx.graphics/src/test/java/test/com/sun/javafx/iio/ImageStorageTest.java
>> - Accept regular file paths, change javadoc
>> - Merge branch 'openjdk:master' into feature/image-datauri
>> - Rename DataURI.isValidURI
>> - Reverted a change
>> - Allow leading and trailing whitespace in data URI
>> - Removed test
>> - Changed data URI detection
>> - Merge branch 'master' into feature/image-datauri
>> - Moved test
>> - ... and 5 more:
>> https://git.openjdk.java.net/jfx/compare/7b7050c4...315523c5
>
> modules/javafx.graphics/src/main/java/javafx/css/converter/URLConverter.java
> line 98:
>
>> 96: }
>> 97:
>> 98: private URL resolve(String stylesheetUrl, String resource) {
>
> Why was this change done? It seems unnecessary and possibly unwanted.
`resource` is never `null` at this point, and it is already trimmed. Also,
contrary to the comment, I cannot see this method being used anywhere in tests,
so I made it private.
-
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
Re: RFR: 8267551: Support loading images from inline data-URIs [v13]
On Fri, 4 Jun 2021 22:14:12 GMT, Kevin Rushforth wrote:
>> Michael Strauß has updated the pull request with a new target base due to a
>> merge or a rebase. The pull request now contains 15 commits:
>>
>> - Merge branch 'master' into feature/image-datauri
>>
>># Conflicts:
>>#
>> modules/javafx.graphics/src/test/java/test/com/sun/javafx/iio/ImageStorageTest.java
>> - Accept regular file paths, change javadoc
>> - Merge branch 'openjdk:master' into feature/image-datauri
>> - Rename DataURI.isValidURI
>> - Reverted a change
>> - Allow leading and trailing whitespace in data URI
>> - Removed test
>> - Changed data URI detection
>> - Merge branch 'master' into feature/image-datauri
>> - Moved test
>> - ... and 5 more:
>> https://git.openjdk.java.net/jfx/compare/7b7050c4...315523c5
>
> modules/javafx.graphics/src/main/java/javafx/scene/image/Image.java line 612:
>
>> 610: /**
>> 611: * Constructs an {@code Image} with content loaded from the
>> specified
>> 612: * image file. The {@code url} parameter can be one of the
>> following:
>
> I would not use the words "image file" in the first sentence. I recommend to
> restore this to "url".
Done.
> modules/javafx.graphics/src/main/java/javafx/scene/image/Image.java line 624:
>
>> 622: *
>> 623: * If a URL uses the "data" scheme, the data must be base64-encoded
>> 624: * and the MIME type must either be empty or a subtype of the
>
> These two paragraphs might be better as part of the class docs (which is
> where we list the supported image formats).
Done.
> modules/javafx.graphics/src/main/java/javafx/scene/image/Image.java line 638:
>
>> 636: /**
>> 637: * Constructs an {@code Image} with content loaded from the
>> specified
>> 638: * image file. The {@code url} parameter can be one of the
>> following:
>
> Similar comment. Maybe something like this for the first sentence?
>
>
> Constructs an {@code Image} with content loaded from the specified url using
> the specified parameters.
Done.
-
PR: https://git.openjdk.java.net/jfx/pull/508
Re: RFR: 8267551: Support loading images from inline data-URIs [v13]
On Fri, 4 Jun 2021 22:06:31 GMT, Kevin Rushforth wrote:
>> Michael Strauß has updated the pull request with a new target base due to a
>> merge or a rebase. The pull request now contains 15 commits:
>>
>> - Merge branch 'master' into feature/image-datauri
>>
>># Conflicts:
>>#
>> modules/javafx.graphics/src/test/java/test/com/sun/javafx/iio/ImageStorageTest.java
>> - Accept regular file paths, change javadoc
>> - Merge branch 'openjdk:master' into feature/image-datauri
>> - Rename DataURI.isValidURI
>> - Reverted a change
>> - Allow leading and trailing whitespace in data URI
>> - Removed test
>> - Changed data URI detection
>> - Merge branch 'master' into feature/image-datauri
>> - Moved test
>> - ... and 5 more:
>> https://git.openjdk.java.net/jfx/compare/7b7050c4...315523c5
>
> modules/javafx.graphics/src/main/java/com/sun/javafx/util/DataURI.java line
> 191:
>
>> 189: public boolean equals(Object o) {
>> 190: if (this == o) return true;
>> 191: if (o == null || getClass() != o.getClass()) return false;
>
> Can this be replaced by `o instanceof DataURI`?
Sure.
-
PR: https://git.openjdk.java.net/jfx/pull/508
Re: RFR: 8267551: Support loading images from inline data-URIs [v13]
On Sat, 5 Jun 2021 15:07:36 GMT, Kevin Rushforth wrote: >> I implemented it in this way so that this requirement is not imposed onto >> the caller, similar to `java.net.URL` does not impose this requirement onto >> its callers. I can imagine that `DataURI` might be used in other places, so >> it might make sense to make it more robust with regards to leading >> whitespace? > > In that case, it might be clearer and simpler to just call `trim()` on the > input String before doing anything with it, unless there is a reason not to. My idea was to avoid repeatedly allocating a new String with the data contained in the URI. Since `matchScheme` is called at least twice (maybe more), that would be a total of at least three copies of the URI data until we parse it. Granted, this would only apply for URIs that contain leading or trailing whitespace (since `trim()` is specified to return the same String instance if it contains no leading or trailing whitespace). - PR: https://git.openjdk.java.net/jfx/pull/508
Re: RFR: 8267551: Support loading images from inline data-URIs [v13]
On Fri, 4 Jun 2021 22:03:26 GMT, Kevin Rushforth wrote:
>> Michael Strauß has updated the pull request with a new target base due to a
>> merge or a rebase. The pull request now contains 15 commits:
>>
>> - Merge branch 'master' into feature/image-datauri
>>
>># Conflicts:
>>#
>> modules/javafx.graphics/src/test/java/test/com/sun/javafx/iio/ImageStorageTest.java
>> - Accept regular file paths, change javadoc
>> - Merge branch 'openjdk:master' into feature/image-datauri
>> - Rename DataURI.isValidURI
>> - Reverted a change
>> - Allow leading and trailing whitespace in data URI
>> - Removed test
>> - Changed data URI detection
>> - Merge branch 'master' into feature/image-datauri
>> - Moved test
>> - ... and 5 more:
>> https://git.openjdk.java.net/jfx/compare/7b7050c4...315523c5
>
> modules/javafx.graphics/src/main/java/com/sun/javafx/util/DataURI.java line
> 175:
>
>> 173:
>> 174: public byte[] getData() {
>> 175: return data;
>
> This returns a reference to the byte array rather than a copy. Is this what
> is wanted? If so, it would be good to add a comment.
I added a comment that clarifies this.
> modules/javafx.graphics/src/main/java/com/sun/javafx/util/DataURI.java line
> 185:
>
>> 183:
>> 184: return originalUri.substring(0, originalUri.length() -
>> originalData.length())
>> 185: + originalData.substring(0, 14) + "..." +
>> originalData.substring(originalData.length() - 14);
>
> Why truncate it?
The idea was to not clutter logs with large pieces of data. Do you think that
the entire data string should be logged?
-
PR: https://git.openjdk.java.net/jfx/pull/508
Re: RFR: 8267551: Support loading images from inline data-URIs [v13]
On Sat, 5 Jun 2021 14:56:15 GMT, Michael Strauß wrote: >> modules/javafx.graphics/src/main/java/com/sun/javafx/util/DataURI.java line >> 47: >> >>> 45: } >>> 46: >>> 47: int firstNonWhitespace = 0, length = uri.length(); >> >> Why do you need to trim leading spaces? The input URL strings should already >> be trimmed by the caller. > > I implemented it in this way so that this requirement is not imposed onto the > caller, similar to `java.net.URL` does not impose this requirement onto its > callers. I can imagine that `DataURI` might be used in other places, so it > might make sense to make it more robust with regards to leading whitespace? In that case, it might be clearer and simpler to just call `trim()` on the input String before doing anything with it, unless there is a reason not to. - PR: https://git.openjdk.java.net/jfx/pull/508
Re: RFR: 8267551: Support loading images from inline data-URIs [v13]
On Fri, 4 Jun 2021 21:59:28 GMT, Kevin Rushforth wrote: >> Michael Strauß has updated the pull request with a new target base due to a >> merge or a rebase. The pull request now contains 15 commits: >> >> - Merge branch 'master' into feature/image-datauri >> >># Conflicts: >># >> modules/javafx.graphics/src/test/java/test/com/sun/javafx/iio/ImageStorageTest.java >> - Accept regular file paths, change javadoc >> - Merge branch 'openjdk:master' into feature/image-datauri >> - Rename DataURI.isValidURI >> - Reverted a change >> - Allow leading and trailing whitespace in data URI >> - Removed test >> - Changed data URI detection >> - Merge branch 'master' into feature/image-datauri >> - Moved test >> - ... and 5 more: >> https://git.openjdk.java.net/jfx/compare/7b7050c4...315523c5 > > modules/javafx.graphics/src/main/java/com/sun/javafx/util/DataURI.java line > 47: > >> 45: } >> 46: >> 47: int firstNonWhitespace = 0, length = uri.length(); > > Why do you need to trim leading spaces? The input URL strings should already > be trimmed by the caller. I implemented it in this way so that this requirement is not imposed onto the caller, similar to `java.net.URL` does not impose this requirement onto its callers. I can imagine that `DataURI` might be used in other places, so it might make sense to make it more robust with regards to leading whitespace? - PR: https://git.openjdk.java.net/jfx/pull/508
Re: RFR: 8267551: Support loading images from inline data-URIs [v13]
On Fri, 4 Jun 2021 21:56:32 GMT, Kevin Rushforth wrote:
>> Michael Strauß has updated the pull request with a new target base due to a
>> merge or a rebase. The pull request now contains 15 commits:
>>
>> - Merge branch 'master' into feature/image-datauri
>>
>># Conflicts:
>>#
>> modules/javafx.graphics/src/test/java/test/com/sun/javafx/iio/ImageStorageTest.java
>> - Accept regular file paths, change javadoc
>> - Merge branch 'openjdk:master' into feature/image-datauri
>> - Rename DataURI.isValidURI
>> - Reverted a change
>> - Allow leading and trailing whitespace in data URI
>> - Removed test
>> - Changed data URI detection
>> - Merge branch 'master' into feature/image-datauri
>> - Moved test
>> - ... and 5 more:
>> https://git.openjdk.java.net/jfx/compare/7b7050c4...315523c5
>
> modules/javafx.graphics/src/main/java/com/sun/javafx/iio/ImageStorage.java
> line 333:
>
>> 331: loader = getLoaderBySignature(theStream, listener);
>> 332: }
>> 333: } catch (Exception e) {
>
> Is this change needed?
If a MIME type other than `image` was specified in the data URL,
`IllegalArgumentException` is thrown. If we only catch `IOException` here, the
IAE will not be wrapped into a `ImageStorageException`.
-
PR: https://git.openjdk.java.net/jfx/pull/508
Re: RFR: 8267551: Support loading images from inline data-URIs [v13]
On Fri, 4 Jun 2021 21:54:12 GMT, Kevin Rushforth wrote:
>> Michael Strauß has updated the pull request with a new target base due to a
>> merge or a rebase. The pull request now contains 15 commits:
>>
>> - Merge branch 'master' into feature/image-datauri
>>
>># Conflicts:
>>#
>> modules/javafx.graphics/src/test/java/test/com/sun/javafx/iio/ImageStorageTest.java
>> - Accept regular file paths, change javadoc
>> - Merge branch 'openjdk:master' into feature/image-datauri
>> - Rename DataURI.isValidURI
>> - Reverted a change
>> - Allow leading and trailing whitespace in data URI
>> - Removed test
>> - Changed data URI detection
>> - Merge branch 'master' into feature/image-datauri
>> - Moved test
>> - ... and 5 more:
>> https://git.openjdk.java.net/jfx/compare/7b7050c4...315523c5
>
> modules/javafx.graphics/src/main/java/com/sun/javafx/iio/ImageStorage.java
> line 290:
>
>> 288: /**
>> 289: * Load all images present in the specified input. For more details
>> refer to
>> 290: * {@link #loadAll(InputStream, ImageLoadListener, double, double,
>> boolean, float, boolean)}.
>
> `ImageLoadListener` isn't imported, so still needs to be qualified (although
> we don't actually generate any docs since this is an implementation class).
`ImageLoadListener` is in the same package as `ImageStorage`, so it should be
implicitly imported.
-
PR: https://git.openjdk.java.net/jfx/pull/508
Re: RFR: 8267551: Support loading images from inline data-URIs
That's a good question, and probably a valid approach. However, while app developers are certainly used to depending on third-party libraries for things like additional controls, I think it's questionable whether it's a good developer experience to needing to rely on the third-party ecosystem for core behavior. We could ship a custom `URLStreamHandler` with the javafx-graphics library, but that presents its own set of problems if app developers use fat jars and have their own implementations for `URLStreamHandlerProvider`. In this case, our implementation might silently fail because it is replaced by an application-provided `META-INF/services/java.net.spi.URLStreamHandlerProvider` file. While this can of course be worked around by using a resource transformer that merges all files of this kind, it is one more non-obvious thing that app developers need to know about. Am Sa., 5. Juni 2021 um 10:17 Uhr schrieb John Hendrikx : > > I have a question about this. > > Why would you make this part of JavaFX directly instead of offering it > as a dependency that people can include in their project? > > As far as I know, it is possible to register custom URL handlers in > Java, which should be used automatically by JavaFX assuming it > constructs its URL's in the standard way when extracting them from the CSS. > > This can be done since Java 9+ via URLStreamHandlerProvider. You need to > store the name of your provider in `META-INF/services`. > > Any reason why this route isn't being taken? > > --John
Re: RFR: 8267551: Support loading images from inline data-URIs [v13]
On Wed, 26 May 2021 16:36:24 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 with a new target base due to a
> merge or a rebase. The pull request now contains 15 commits:
>
> - Merge branch 'master' into feature/image-datauri
>
># Conflicts:
>#
> modules/javafx.graphics/src/test/java/test/com/sun/javafx/iio/ImageStorageTest.java
> - Accept regular file paths, change javadoc
> - Merge branch 'openjdk:master' into feature/image-datauri
> - Rename DataURI.isValidURI
> - Reverted a change
> - Allow leading and trailing whitespace in data URI
> - Removed test
> - Changed data URI detection
> - Merge branch 'master' into feature/image-datauri
> - Moved test
> - ... and 5 more:
> https://git.openjdk.java.net/jfx/compare/7b7050c4...315523c5
modules/javafx.graphics/src/main/java/com/sun/javafx/css/StyleManager.java line
776:
> 774: } else {
> 775: logger.warning("Error loading image:
> " + url);
> 776: }
So if I understand this correctly, an Image is first loaded via the standard
mechanism, and if that fails for any reason (`image.isError()`) a second
attempt is made through `DataURI` ?
Would it not be better to register a new protocol so `new Image` would just
succeed for data uri's ?
-
PR: https://git.openjdk.java.net/jfx/pull/508
Re: RFR: 8267551: Support loading images from inline data-URIs
I have a question about this. Why would you make this part of JavaFX directly instead of offering it as a dependency that people can include in their project? As far as I know, it is possible to register custom URL handlers in Java, which should be used automatically by JavaFX assuming it constructs its URL's in the standard way when extracting them from the CSS. This can be done since Java 9+ via URLStreamHandlerProvider. You need to store the name of your provider in `META-INF/services`. Any reason why this route isn't being taken? --John On 21/05/2021 19:58, 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. - Commit messages: - Added data-URI loading support for images Changes: https://git.openjdk.java.net/jfx/pull/508/files Webrev: https://webrevs.openjdk.java.net/?repo=jfx=508=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8267551 Stats: 437 lines in 8 files changed: 414 ins; 9 del; 14 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
Re: RFR: 8267551: Support loading images from inline data-URIs [v13]
On Wed, 26 May 2021 16:36:24 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 with a new target base due to a
> merge or a rebase. The pull request now contains 15 commits:
>
> - Merge branch 'master' into feature/image-datauri
>
># Conflicts:
>#
> modules/javafx.graphics/src/test/java/test/com/sun/javafx/iio/ImageStorageTest.java
> - Accept regular file paths, change javadoc
> - Merge branch 'openjdk:master' into feature/image-datauri
> - Rename DataURI.isValidURI
> - Reverted a change
> - Allow leading and trailing whitespace in data URI
> - Removed test
> - Changed data URI detection
> - Merge branch 'master' into feature/image-datauri
> - Moved test
> - ... and 5 more:
> https://git.openjdk.java.net/jfx/compare/7b7050c4...315523c5
I haven't tested this yet, but have done a first pass on the spec changes and
most of the implementation (I'll finish next week).
Overall, the docs look good. Can you also update the list of supported image
formats in the `Image` class docs just before line 76?
modules/javafx.graphics/src/main/java/com/sun/javafx/iio/ImageStorage.java line
290:
> 288: /**
> 289: * Load all images present in the specified input. For more details
> refer to
> 290: * {@link #loadAll(InputStream, ImageLoadListener, double, double,
> boolean, float, boolean)}.
`ImageLoadListener` isn't imported, so still needs to be qualified (although we
don't actually generate any docs since this is an implementation class).
modules/javafx.graphics/src/main/java/com/sun/javafx/iio/ImageStorage.java line
333:
> 331: loader = getLoaderBySignature(theStream, listener);
> 332: }
> 333: } catch (Exception e) {
Is this change needed?
modules/javafx.graphics/src/main/java/com/sun/javafx/util/DataURI.java line 47:
> 45: }
> 46:
> 47: int firstNonWhitespace = 0, length = uri.length();
Why do you need to trim leading spaces? The input URL strings should already be
trimmed by the caller.
modules/javafx.graphics/src/main/java/com/sun/javafx/util/DataURI.java line 76:
> 74:
> 75: if (Character.isWhitespace(uri.charAt(0))) {
> 76: uri = uri.trim();
Same question as above.
modules/javafx.graphics/src/main/java/com/sun/javafx/util/DataURI.java line 175:
> 173:
> 174: public byte[] getData() {
> 175: return data;
This returns a reference to the byte array rather than a copy. Is this what is
wanted? If so, it would be good to add a comment.
modules/javafx.graphics/src/main/java/com/sun/javafx/util/DataURI.java line 185:
> 183:
> 184: return originalUri.substring(0, originalUri.length() -
> originalData.length())
> 185: + originalData.substring(0, 14) + "..." +
> originalData.substring(originalData.length() - 14);
Why truncate it?
modules/javafx.graphics/src/main/java/com/sun/javafx/util/DataURI.java line 191:
> 189: public boolean equals(Object o) {
> 190: if (this == o) return true;
> 191: if (o == null || getClass() != o.getClass()) return false;
Can this be replaced by `o instanceof DataURI`?
modules/javafx.graphics/src/main/java/javafx/css/converter/URLConverter.java
line 98:
> 96: }
> 97:
> 98: private URL resolve(String stylesheetUrl, String resource) {
Why was this change done? It seems unnecessary and possibly unwanted.
modules/javafx.graphics/src/main/java/javafx/scene/image/Image.java line 612:
> 610: /**
> 611: * Constructs an {@code Image} with content loaded from the specified
> 612: * image file. The {@code url} parameter can be one of the following:
I would not use the words "image file" in the first sentence. I recommend to
restore this to "url".
modules/javafx.graphics/src/main/java/javafx/scene/image/Image.java line 624:
> 622: *
> 623: * If a URL uses the "data" scheme, the data must be base64-encoded
> 624: * and the MIME type must either be empty or a subtype of the
These two paragraphs might be better as part of the class docs (which is where
we list the supported image formats).
modules/javafx.graphics/src/main/java/javafx/scene/image/Image.java line 638:
> 636: /**
> 637: * Constructs an {@code Image} with content loaded from the specified
> 638: * image file. The {@code url} parameter can be one of the following:
Similar comment. Maybe something like this for the first sentence?
Constructs an {@code Image} with content loaded from the specified url using
the specified parameters.
-
PR: https://git.openjdk.java.net/jfx/pull/508
Re: RFR: 8267551: Support loading images from inline data-URIs [v13]
> 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 with a new target base due to a merge or a rebase. The pull request now contains 15 commits: - Merge branch 'master' into feature/image-datauri # Conflicts: # modules/javafx.graphics/src/test/java/test/com/sun/javafx/iio/ImageStorageTest.java - Accept regular file paths, change javadoc - Merge branch 'openjdk:master' into feature/image-datauri - Rename DataURI.isValidURI - Reverted a change - Allow leading and trailing whitespace in data URI - Removed test - Changed data URI detection - Merge branch 'master' into feature/image-datauri - Moved test - ... and 5 more: https://git.openjdk.java.net/jfx/compare/7b7050c4...315523c5 - Changes: https://git.openjdk.java.net/jfx/pull/508/files Webrev: https://webrevs.openjdk.java.net/?repo=jfx=508=12 Stats: 566 lines in 8 files changed: 501 ins; 23 del; 42 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
Re: RFR: 8267551: Support loading images from inline data-URIs [v12]
> 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: Accept regular file paths, change javadoc - Changes: - all: https://git.openjdk.java.net/jfx/pull/508/files - new: https://git.openjdk.java.net/jfx/pull/508/files/f6878f51..d30f6109 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jfx=508=11 - incr: https://webrevs.openjdk.java.net/?repo=jfx=508=10-11 Stats: 30 lines in 1 file changed: 18 ins; 0 del; 12 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
Re: RFR: 8267551: Support loading images from inline data-URIs [v11]
> 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 with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains 13 additional commits since the last revision: - Merge branch 'openjdk:master' into feature/image-datauri - Rename DataURI.isValidURI - Reverted a change - Allow leading and trailing whitespace in data URI - Removed test - Changed data URI detection - Merge branch 'master' into feature/image-datauri - Moved test - Added javadoc - Added javadoc - ... and 3 more: https://git.openjdk.java.net/jfx/compare/fc92ab9f...f6878f51 - Changes: - all: https://git.openjdk.java.net/jfx/pull/508/files - new: https://git.openjdk.java.net/jfx/pull/508/files/edb6f658..f6878f51 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jfx=508=10 - incr: https://webrevs.openjdk.java.net/?repo=jfx=508=09-10 Stats: 559 lines in 12 files changed: 386 ins; 145 del; 28 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
Re: RFR: 8267551: Support loading images from inline data-URIs [v10]
> 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: Rename DataURI.isValidURI - Changes: - all: https://git.openjdk.java.net/jfx/pull/508/files - new: https://git.openjdk.java.net/jfx/pull/508/files/30211a4c..edb6f658 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jfx=508=09 - incr: https://webrevs.openjdk.java.net/?repo=jfx=508=08-09 Stats: 15 lines in 4 files changed: 0 ins; 0 del; 15 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
Re: RFR: 8267551: Support loading images from inline data-URIs [v9]
> 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: Reverted a change - Changes: - all: https://git.openjdk.java.net/jfx/pull/508/files - new: https://git.openjdk.java.net/jfx/pull/508/files/d707615e..30211a4c Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jfx=508=08 - incr: https://webrevs.openjdk.java.net/?repo=jfx=508=07-08 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 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
Re: RFR: 8267551: Support loading images from inline data-URIs [v8]
> 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 two additional commits since the last revision: - Allow leading and trailing whitespace in data URI - Removed test - Changes: - all: https://git.openjdk.java.net/jfx/pull/508/files - new: https://git.openjdk.java.net/jfx/pull/508/files/907bc782..d707615e Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jfx=508=07 - incr: https://webrevs.openjdk.java.net/?repo=jfx=508=06-07 Stats: 35 lines in 3 files changed: 27 ins; 6 del; 2 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
Re: RFR: 8267551: Support loading images from inline data-URIs [v7]
> 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: Changed data URI detection - Changes: - all: https://git.openjdk.java.net/jfx/pull/508/files - new: https://git.openjdk.java.net/jfx/pull/508/files/4302e9f1..907bc782 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jfx=508=06 - incr: https://webrevs.openjdk.java.net/?repo=jfx=508=05-06 Stats: 65 lines in 6 files changed: 12 ins; 25 del; 28 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
Re: RFR: 8267551: Support loading images from inline data-URIs [v6]
> 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 with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains seven additional commits since the last revision: - Merge branch 'master' into feature/image-datauri - Moved test - Added javadoc - Added javadoc - Added javadoc - Added javadoc - Added data-URI loading support for images - Changes: - all: https://git.openjdk.java.net/jfx/pull/508/files - new: https://git.openjdk.java.net/jfx/pull/508/files/3a77ac0b..4302e9f1 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jfx=508=05 - incr: https://webrevs.openjdk.java.net/?repo=jfx=508=04-05 Stats: 683 lines in 52 files changed: 400 ins; 98 del; 185 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
Re: RFR: 8267551: Support loading images from inline data-URIs [v5]
> 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: Moved test - Changes: - all: https://git.openjdk.java.net/jfx/pull/508/files - new: https://git.openjdk.java.net/jfx/pull/508/files/07ff1aa4..3a77ac0b Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jfx=508=04 - incr: https://webrevs.openjdk.java.net/?repo=jfx=508=03-04 Stats: 44 lines in 3 files changed: 24 ins; 18 del; 2 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
Re: RFR: 8267551: Support loading images from inline data-URIs [v4]
> 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 two additional commits since the last revision: - Added javadoc - Added javadoc - Changes: - all: https://git.openjdk.java.net/jfx/pull/508/files - new: https://git.openjdk.java.net/jfx/pull/508/files/71eb2d20..07ff1aa4 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jfx=508=03 - incr: https://webrevs.openjdk.java.net/?repo=jfx=508=02-03 Stats: 49 lines in 1 file changed: 16 ins; 11 del; 22 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
Re: RFR: 8267551: Support loading images from inline data-URIs [v3]
> 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: Added javadoc - Changes: - all: https://git.openjdk.java.net/jfx/pull/508/files - new: https://git.openjdk.java.net/jfx/pull/508/files/e23ab085..71eb2d20 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jfx=508=02 - incr: https://webrevs.openjdk.java.net/?repo=jfx=508=01-02 Stats: 4 lines in 1 file changed: 4 ins; 0 del; 0 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
Re: RFR: 8267551: Support loading images from inline data-URIs [v2]
> 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: Added javadoc - Changes: - all: https://git.openjdk.java.net/jfx/pull/508/files - new: https://git.openjdk.java.net/jfx/pull/508/files/4f273d0f..e23ab085 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jfx=508=01 - incr: https://webrevs.openjdk.java.net/?repo=jfx=508=00-01 Stats: 65 lines in 3 files changed: 40 ins; 8 del; 17 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
Re: RFR: 8267551: Support loading images from inline data-URIs
On Wed, 19 May 2021 20:24:31 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. I'd like to see this documented somewhere in the `Image` class docs. - PR: https://git.openjdk.java.net/jfx/pull/508
RFR: 8267551: Support loading images from inline data-URIs
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. - Commit messages: - Added data-URI loading support for images Changes: https://git.openjdk.java.net/jfx/pull/508/files Webrev: https://webrevs.openjdk.java.net/?repo=jfx=508=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8267551 Stats: 437 lines in 8 files changed: 414 ins; 9 del; 14 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
