RFR: 8276206: Rename TextBinding class to better express its purpose
This PR renames `TextBinding` to `MnemonicInfo` and adds the following text to its javadoc: /** 33 +* Provides information about mnemonics contained within a string. - Commit messages: - Rename TextBinding to MnemonicInfo Changes: https://git.openjdk.java.net/jfx/pull/678/files Webrev: https://webrevs.openjdk.java.net/?repo=jfx&pr=678&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8276206 Stats: 320 lines in 6 files changed: 146 ins; 146 del; 28 mod Patch: https://git.openjdk.java.net/jfx/pull/678.diff Fetch: git fetch https://git.openjdk.java.net/jfx pull/678/head:pull/678 PR: https://git.openjdk.java.net/jfx/pull/678
Re: RFR: 8277572: ImageStorage should correctly handle MIME types for images encoded in data URIs [v4]
> `com.sun.javafx.iio.ImageStorage` currently ignores the MIME image subtype
> specified for images encoded in data URIs. This should be improved as follows:
>
> 1. If the specified image subtype is not supported, an exception will be
> thrown.
> 2. If the specified image subtype is supported, but the data contained in the
> URI is of a different (but also supported) image format, the image will be
> loaded and a warning will be logged. For example, if the MIME type is
> "image/jpeg", but the image data is PNG, the following warning will be
> generated:
>
>
> Image format 'PNG' does not match MIME type 'image/jpeg' in URI
> 'data:image/jpeg;base64,iVBORw0KGgoAAA...AAAElFTkSuQmCC'
>
>
> Also, the javadoc of `javafx.scene.image.Image` incorrectly states:
>
> 94* If a URL uses the "data" scheme, the data must be base64-encoded
> 95* and the MIME type must either be empty or a subtype of the
> 96* {@code image} type.
>
> However, omitting the MIME type of a data URI is specified to imply
> "text/plain" (RFC 2397, section 2). Since the `com.sun.javafx.util.DataURI`
> class is implemented according to this specification, trying to load an image
> without MIME type correctly fails with an `ImageStorageException`:
> "Unexpected MIME type: text".
>
> The solution is to fix the documentation:
>
> 94* If a URL uses the "data" scheme, the data must be base64-encoded
> - 95* and the MIME type must either be empty or a subtype of the
> - 96* {@code image} type.
> + 95* and the MIME type must be a subtype of the {@code image} type.
> + 96* The MIME type must match the image format of the data contained
> in
> + 97* the URL. In case of a mismatch between MIME type and image
> format,
> + 98* the image will be loaded if the image format can be determined
> by
> + 99* JavaFX, and a warning will be logged.
Michael Strauß has updated the pull request incrementally with one additional
commit since the last revision:
Added test for image format without signature
-
Changes:
- all: https://git.openjdk.java.net/jfx/pull/676/files
- new: https://git.openjdk.java.net/jfx/pull/676/files/0167532b..4cb5678a
Webrevs:
- full: https://webrevs.openjdk.java.net/?repo=jfx&pr=676&range=03
- incr: https://webrevs.openjdk.java.net/?repo=jfx&pr=676&range=02-03
Stats: 89 lines in 8 files changed: 51 ins; 3 del; 35 mod
Patch: https://git.openjdk.java.net/jfx/pull/676.diff
Fetch: git fetch https://git.openjdk.java.net/jfx pull/676/head:pull/676
PR: https://git.openjdk.java.net/jfx/pull/676
Re: RFR: 8274274: Update JUnit to version 5.8.1 [v9]
On Mon, 22 Nov 2021 13:43:34 GMT, John Hendrikx wrote: >> I've added JUnit 5 as a test dependency and made sure that the JUnit 4 tests >> still work. Also added a single JUnit 5 tests, and confirmed it works. >> >> I've updated the Eclipse project file for the base module only. > > John Hendrikx has updated the pull request incrementally with two additional > commits since the last revision: > > - Remove remaining references to old JUnit 4.8.2 > - Upgrade to JUnit 4.13.2 The changes look good to me. The tests work as expected. - Marked as reviewed by mstrauss (Author). PR: https://git.openjdk.java.net/jfx/pull/633
Re: RFR: 8197991: Selecting many items in a TableView is very slow
On Wed, 17 Nov 2021 05:34:46 GMT, Abhinay Agarwal wrote: > This work improves the performance of `MultipleSelectionModel` over large > data sets by caching some values and avoiding unnecessary calls to > `SelectedIndicesList#size`. It further improves the performance by reducing > the number of iterations required to find the index of an element in the > BitSet. > > The work is based on [an abandoned > patch](https://github.com/openjdk/jfx/pull/127) submitted by @yososs > > There are currently 2 manual tests for this fix. I see that the `/contributor` command didn't work with the original contributor's GitHub username. You can instead take the contributor information from the HEAD commit of the pull request: `Naohiro Yoshimoto yosb...@gmail.com`. - PR: https://git.openjdk.java.net/jfx/pull/673
Re: RFR: 8277572: ImageStorage should correctly handle MIME types for images encoded in data URIs [v2]
On Tue, 23 Nov 2021 15:33:49 GMT, Kevin Rushforth wrote: > This seems like an odd behavior to me, but if this is common practice among > browsers, then it seems OK for us to do the same. In this case, it should be > documented in the class docs of `Image`. I've tested this with Chrome and Edge, and these browsers go even further. If the image format has a detectable signature, the image will be displayed regardless of the specified MIME type (even if it is not an `image` type). The MIME type only matters for image formats without signature, like SVG. In this case, the image will not be displayed if the MIME type is incorrect. I think that a compromise works best for JavaFX. We probably don't want to allow plainly incorrect MIME types, but in cases where we know what the developer should have specified, a warning seems like a good nudge in the right direction. - PR: https://git.openjdk.java.net/jfx/pull/676
Re: RFR: 8277572: ImageStorage should correctly handle MIME types for images encoded in data URIs [v3]
> `com.sun.javafx.iio.ImageStorage` currently ignores the MIME image subtype
> specified for images encoded in data URIs. This should be improved as follows:
>
> 1. If the specified image subtype is not supported, an exception will be
> thrown.
> 2. If the specified image subtype is supported, but the data contained in the
> URI is of a different (but also supported) image format, the image will be
> loaded and a warning will be logged. For example, if the MIME type is
> "image/jpeg", but the image data is PNG, the following warning will be
> generated:
>
>
> Image format 'PNG' does not match MIME type 'image/jpeg' in URI
> 'data:image/jpeg;base64,iVBORw0KGgoAAA...AAAElFTkSuQmCC'
>
>
> Also, the javadoc of `javafx.scene.image.Image` incorrectly states:
>
> 94* If a URL uses the "data" scheme, the data must be base64-encoded
> 95* and the MIME type must either be empty or a subtype of the
> 96* {@code image} type.
>
> However, omitting the MIME type of a data URI is specified to imply
> "text/plain" (RFC 2397, section 2). Since the `com.sun.javafx.util.DataURI`
> class is implemented according to this specification, trying to load an image
> without MIME type correctly fails with an `ImageStorageException`:
> "Unexpected MIME type: text".
>
> The solution is to fix the documentation:
>
> 94* If a URL uses the "data" scheme, the data must be base64-encoded
> + 95* and the MIME type must be a subtype of the {@code image} type.
> - 95* and the MIME type must either be empty or a subtype of the
> - 96* {@code image} type.
Michael Strauß has updated the pull request incrementally with one additional
commit since the last revision:
Added documentation for MIME/data mismatch
-
Changes:
- all: https://git.openjdk.java.net/jfx/pull/676/files
- new: https://git.openjdk.java.net/jfx/pull/676/files/73f1bb13..0167532b
Webrevs:
- full: https://webrevs.openjdk.java.net/?repo=jfx&pr=676&range=02
- incr: https://webrevs.openjdk.java.net/?repo=jfx&pr=676&range=01-02
Stats: 4 lines in 1 file changed: 4 ins; 0 del; 0 mod
Patch: https://git.openjdk.java.net/jfx/pull/676.diff
Fetch: git fetch https://git.openjdk.java.net/jfx pull/676/head:pull/676
PR: https://git.openjdk.java.net/jfx/pull/676
Re: RFR: 8254956: [REDO] Memoryleak: Closed focused Stages are not collected with Monocle
On Tue, 23 Nov 2021 11:00:51 GMT, Jose Pereda wrote: > This PR fixes a memory leak, as a follow-up of > [JDK-8241840](https://bugs.openjdk.java.net/browse/JDK-8241840). > > The tests pass on desktop (Linux/macOS/Windows) and there is no regression on > Android (see [JDK-8254605](https://bugs.openjdk.java.net/browse/JDK-8254605)). Yes, `cleanTest` is quite handy when you are just modifying the code base and running the same test all over again (else you need to modify the test just a tiny bit...) - PR: https://git.openjdk.java.net/jfx/pull/677
Integrated: 8254956: [REDO] Memoryleak: Closed focused Stages are not collected with Monocle
On Tue, 23 Nov 2021 11:00:51 GMT, Jose Pereda wrote: > This PR fixes a memory leak, as a follow-up of > [JDK-8241840](https://bugs.openjdk.java.net/browse/JDK-8241840). > > The tests pass on desktop (Linux/macOS/Windows) and there is no regression on > Android (see [JDK-8254605](https://bugs.openjdk.java.net/browse/JDK-8254605)). This pull request has now been integrated. Changeset: e694fb55 Author:Jose Pereda URL: https://git.openjdk.java.net/jfx/commit/e694fb55692d3d438a947e2b0c307083e033ab30 Stats: 20 lines in 4 files changed: 11 ins; 6 del; 3 mod 8254956: [REDO] Memoryleak: Closed focused Stages are not collected with Monocle Reviewed-by: kcr - PR: https://git.openjdk.java.net/jfx/pull/677
Re: RFR: 8254956: [REDO] Memoryleak: Closed focused Stages are not collected with Monocle
On Tue, 23 Nov 2021 11:00:51 GMT, Jose Pereda wrote: > This PR fixes a memory leak, as a follow-up of > [JDK-8241840](https://bugs.openjdk.java.net/browse/JDK-8241840). > > The tests pass on desktop (Linux/macOS/Windows) and there is no regression on > Android (see [JDK-8254605](https://bugs.openjdk.java.net/browse/JDK-8254605)). This was my mistake -- well, actually, an unfortunate design choice by gradle, but one I'm so familiar with I'm surprised I got bit by it. gradle will not rerun a test if it thinks the tests are up-to-date, and that's exactly what happened to me. Twice. This is why when I run individual tests, I (almost) always add the `cleanTest` target. When I do that, it fails for me, too, without your fix. - Marked as reviewed by kcr (Lead). PR: https://git.openjdk.java.net/jfx/pull/677
Re: RFR: 8254956: [REDO] Memoryleak: Closed focused Stages are not collected with Monocle
On Tue, 23 Nov 2021 11:00:51 GMT, Jose Pereda wrote: > This PR fixes a memory leak, as a follow-up of > [JDK-8241840](https://bugs.openjdk.java.net/browse/JDK-8241840). > > The tests pass on desktop (Linux/macOS/Windows) and there is no regression on > Android (see [JDK-8254605](https://bugs.openjdk.java.net/browse/JDK-8254605)). Hmm, it does fail for me without the fix. For instance, on Mac (Big Sur), with JDK 11 or 16, and building from head: sh gradlew -PFULL_TEST=true :systemTests:test --info --tests=test.javafx.stage.FocusedWindowMonocleTest test.javafx.stage.FocusedWindowMonocleTest > testClosedFocusedStageLeak FAILED junit.framework.AssertionFailedError: Expected: but was: javafx.stage.Stage@6caf0677 at junit.framework.Assert.fail(Assert.java:47) at junit.framework.Assert.assertTrue(Assert.java:20) at junit.framework.Assert.assertNull(Assert.java:233) at junit.framework.Assert.assertNull(Assert.java:226) at test.javafx.stage.FocusedWindowTestBase.assertCollectable(FocusedWindowTestBase.java:97) at test.javafx.stage.FocusedWindowTestBase.testClosedFocusedStageLeakBase(FocusedWindowTestBase.java:81) at test.javafx.stage.FocusedWindowMonocleTest.testClosedFocusedStageLeak(FocusedWindowMonocleTest.java:48) - PR: https://git.openjdk.java.net/jfx/pull/677
Re: RFR: 8277572: ImageStorage should correctly handle MIME types for images encoded in data URIs [v2]
On Tue, 23 Nov 2021 05:50:33 GMT, Michael Strauß wrote:
>> `com.sun.javafx.iio.ImageStorage` currently ignores the MIME image subtype
>> specified for images encoded in data URIs. This should be improved as
>> follows:
>>
>> 1. If the specified image subtype is not supported, an exception will be
>> thrown.
>> 2. If the specified image subtype is supported, but the data contained in
>> the URI is of a different (but also supported) image format, the image will
>> be loaded and a warning will be logged. For example, if the MIME type is
>> "image/jpeg", but the image data is PNG, the following warning will be
>> generated:
>>
>>
>> Image format 'PNG' does not match MIME type 'image/jpeg' in URI
>> 'data:image/jpeg;base64,iVBORw0KGgoAAA...AAAElFTkSuQmCC'
>>
>>
>> Also, the javadoc of `javafx.scene.image.Image` incorrectly states:
>>
>> 94* If a URL uses the "data" scheme, the data must be base64-encoded
>> 95* and the MIME type must either be empty or a subtype of the
>> 96* {@code image} type.
>>
>> However, omitting the MIME type of a data URI is specified to imply
>> "text/plain" (RFC 2397, section 2). Since the `com.sun.javafx.util.DataURI`
>> class is implemented according to this specification, trying to load an
>> image without MIME type correctly fails with an `ImageStorageException`:
>> "Unexpected MIME type: text".
>>
>> The solution is to fix the documentation:
>>
>> 94* If a URL uses the "data" scheme, the data must be
>> base64-encoded
>> + 95* and the MIME type must be a subtype of the {@code image} type.
>> - 95* and the MIME type must either be empty or a subtype of the
>> - 96* {@code image} type.
>
> Michael Strauß has updated the pull request incrementally with one additional
> commit since the last revision:
>
> ImageStorage correctly handles MIME types in data URIs
> 2. If the specified image subtype is supported, but the data contained in the
> URI is of a different (but also supported) image format, the image will be
> loaded and a warning will be logged. For example, if the MIME type is
> "image/jpeg", but the image data is PNG, the following warning will be
> generated:
>
> ```
> Image format 'PNG' does not match MIME type 'image/jpeg' in URI
> 'data:image/jpeg;base64,iVBORw0KGgoAAA...AAAElFTkSuQmCC'
> ```
This seems like an odd behavior to me, but if this is common practice among
browsers, then it seems OK for us to do the same. In this case, it should be
documented in the class docs of `Image`.
-
PR: https://git.openjdk.java.net/jfx/pull/676
Re: RFR: 8254956: [REDO] Memoryleak: Closed focused Stages are not collected with Monocle
On Tue, 23 Nov 2021 11:00:51 GMT, Jose Pereda wrote: > This PR fixes a memory leak, as a follow-up of > [JDK-8241840](https://bugs.openjdk.java.net/browse/JDK-8241840). > > The tests pass on desktop (Linux/macOS/Windows) and there is no regression on > Android (see [JDK-8254605](https://bugs.openjdk.java.net/browse/JDK-8254605)). The fix looks good to me. The newly reenabled test doesn't seem to verify the fix, though. It passes both before and after the fix on two different systems (possibly because some other recent change has made it insufficient to test for the leak). Do you have a test case that fails without this redo fix and passes with it? - PR: https://git.openjdk.java.net/jfx/pull/677
RFR: 8254956: [REDO] Memoryleak: Closed focused Stages are not collected with Monocle
This PR fixes a memory leak, as a follow-up of [JDK-8241840](https://bugs.openjdk.java.net/browse/JDK-8241840). The tests pass on desktop (Linux/macOS/Windows) and there is no regression on Android (see [JDK-8254605](https://bugs.openjdk.java.net/browse/JDK-8254605)). - Commit messages: - Enable test back and include required sw pipeline - Nullify focusedWindow when window is closed and add null checks Changes: https://git.openjdk.java.net/jfx/pull/677/files Webrev: https://webrevs.openjdk.java.net/?repo=jfx&pr=677&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8254956 Stats: 20 lines in 4 files changed: 11 ins; 6 del; 3 mod Patch: https://git.openjdk.java.net/jfx/pull/677.diff Fetch: git fetch https://git.openjdk.java.net/jfx pull/677/head:pull/677 PR: https://git.openjdk.java.net/jfx/pull/677
Re: RFR: 8276847: JSException: ReferenceError: Can't find variable: IntersectionObserver
On Wed, 10 Nov 2021 18:20:11 GMT, Jose Pereda wrote: > This PR enables intersection observer support for WebKit, and includes a > system test. Worth pointing out that [JDK-8273558](https://bugs.openjdk.java.net/browse/JDK-8273558) gets fixed with this PR. - PR: https://git.openjdk.java.net/jfx/pull/664
