RFR: 8264330: Scene MouseHandler is referencing removed nodes
Small fix to clear a reference to a removed node left by Scene$MouseHandler. - Commit messages: - 8264330: Scene MouseHandler is referencing removed nodes Changes: https://git.openjdk.java.net/jfx/pull/448/files Webrev: https://webrevs.openjdk.java.net/?repo=jfx=448=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8264330 Stats: 52 lines in 2 files changed: 51 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jfx/pull/448.diff Fetch: git fetch https://git.openjdk.java.net/jfx pull/448/head:pull/448 PR: https://git.openjdk.java.net/jfx/pull/448
RFR: 8262366: Update glib to version 2.66.7
- GLib was updated to version 2.66.7 and GStreamer to version 1.18.3 - One bug was discovered in updated GStreamer which was causing deadlock or infinite loop during seek on macOS. See gstsystemclock.c for changes between ifdef GSTREAMER_LITE. Otherwise no changes. - Commit messages: - 8262366: Update glib to version 2.66.7 Changes: https://git.openjdk.java.net/jfx/pull/447/files Webrev: https://webrevs.openjdk.java.net/?repo=jfx=447=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8262366 Stats: 39553 lines in 439 files changed: 23426 ins; 5427 del; 10700 mod Patch: https://git.openjdk.java.net/jfx/pull/447.diff Fetch: git fetch https://git.openjdk.java.net/jfx pull/447/head:pull/447 PR: https://git.openjdk.java.net/jfx/pull/447
Re: RFR: 8242508: Upgrade to Visual Studio 2019 version 16.5.3
On Thu, 25 Mar 2021 22:58:57 GMT, Kevin Rushforth wrote: > Build fails on my system with `FAIL: WINSDK_DIR not defined` @palexdev I hit that error, too. You may have already found this, but you can work around the problem by using the sample [`windows_tools.properties`][1] file on the Building OpenJFX page (click "Expand source"). You just have to adjust the `WINDOWS_VS_PATH` and remember to copy the file to `build/windows_tools.properties` after running `gradle clean` and before running a build. [1]: https://wiki.openjdk.java.net/display/OpenJFX/Building+OpenJFX#BuildingOpenJFX-Missingpathsissue - PR: https://git.openjdk.java.net/jfx/pull/212
Re: RFR: 8258777: SkinBase: add api to un-/register invalidation-/listChange listeners [v2]
On Mon, 29 Mar 2021 15:39:58 GMT, Jeanette Winzenburg
wrote:
>> modules/javafx.controls/src/main/java/javafx/scene/control/SkinBase.java
>> line 268:
>>
>>> 266: *
>>> 267: * @param observable The observable for which all listeners should
>>> be removed.
>>> 268: * @return A single chained {@link Consumer} consisting of all
>>> {@link Consumer consumers} registered through
>>
>> 1. There's no need for a `@link` on a class that is in the arguments or
>> return of the method since they are linked there anyway, and it is
>> recommended to use `@link` only the first time the class appears.
>>
>> 2. You might want to clarify that by "chained" you mean that they were
>> composed using `Consumer#andThen`, either using `@plainlink` on "chained",
>> or explicitly by "chained using Consumer#andThen`. Then again, it might be
>> obvious.
>
> - kept only the link to the registerXX method (to clarify the scope of the
> removal)
> - replaced "chained" by "composed"
> - concentrated on what that composed consumer does (performs all removed
> operations)
>
> Not entirely certain whether it's clear enough now
>
> - should the description have an additional sentence `Returns a composed
> [same-as-in-returns]` Undecided.
> - should the `same-as-in-returns` contain the specification of the sequence
> of performing (from the register method)? Tend to no (because it's getting
> too long), but then ..
I haven't gone back and done a detailed review yet, but I like the overall
changes. The one thing I did notice is that the language used to describe the
return value of `unregisterInvalidationListeners` and
`unregisterListChangeListeners` is different:
unregisterInvalidationListeners:
* @return a composed consumer that performs all removed operations, or
* {@code null} if none have been registered or the observable is {@code
null}
unregisterListChangeListeners:
* @return A single chained {@link Consumer} consisting of all {@link
Consumer consumers} registered through
* {@link #registerListChangeListener(ObservableList, Consumer)}. If
no consumers have been registered on this
* list, null will be returned.
I find it confusing to say that the returned list "performs all removed
operations", so I like the language in the latter method better. Some might
interpret the former to mean that is is somehow necessary to call the returned
chained consumer as part of the removal, which isn't what you meant to say.
-
PR: https://git.openjdk.java.net/jfx/pull/409
Re: RFR: 8258777: SkinBase: add api to un-/register invalidation-/listChange listeners [v2]
On Tue, 30 Mar 2021 13:31:36 GMT, Jeanette Winzenburg wrote: >>> hmm ... failing checks, why? >> >> The failure on Windows is because your branch isn't up-to-date with >> `master`, and is missing a recent fix to the Window build. You can either >> `git merge master` (after updating your `master` branch from the upstream >> repo) or ignore the error. > >> >> The failure on Windows is because your branch isn't up-to-date with >> `master`, and is missing a recent fix to the Window build. You can either >> `git merge master` (after updating your `master` branch from the upstream >> repo) or ignore the error. > > thanks for the info - will ignore for now, the merge/rebase will happen after > approval anyway :) I think that the docs are ready for the CSR now. - PR: https://git.openjdk.java.net/jfx/pull/409
Re: RFR: 8264449: Enable reproducible builds with SOURCE_DATE_EPOCH
On Tue, 30 Mar 2021 16:30:25 GMT, John Neffenger wrote: > I think there might be a Skara bug. No, no bug. Sorry about that. I just don't know how GitHub works. :frowning_face: The pre-submit tests ran two days ago when I pushed the branch to GitHub, so by the time I submitted the pull request, they had finished long ago. - PR: https://git.openjdk.java.net/jfx/pull/446
Re: E-mail addresses at openjdk.org
On 3/25/21 4:12 AM, Kevin Rushforth wrote: Until then, adding the email address as an unverified additional email address in your GitHub profile is the way to go. And the first GitHub user to claim the address wins! There are a large number of unclaimed commits in our repository that are up for grabs, as far as GitHub is concerned. Worse yet, the GitHub website masks the real name and e-mail address in the commit with the name of the GitHub user who claimed it. It's not a security issue for the project or any of its contributors, but I suppose it could be used as a form of identify theft. It takes just a few seconds to add your openjdk.org address, if you have one, to your GitHub profile. John
Re: issues on javafxports/openjdk-jfx
Would archiving the repository work for this? [1] -- Kevin [1] https://docs.github.com/en/github/creating-cloning-and-archiving-repositories/archiving-a-github-repository On 3/30/2021 11:16 AM, John Neffenger wrote: On 3/28/21 5:24 AM, Johan Vos wrote: I'm still in favor of closing it though. It would be nice if there were a way in GitHub to close the repository to new issues but keep the old ones public and editable. For example, I'm still getting good information on the old closed issues, even as recently as last week: mgroth0 commented 8 days ago https://github.com/javafxports/openjdk-jfx/issues/229#issuecomment-804546080 That doesn't seem possible, though. I can find only a checkbox to enable or disable issues. John
Re: issues on javafxports/openjdk-jfx
On 3/28/21 5:24 AM, Johan Vos wrote: I'm still in favor of closing it though. It would be nice if there were a way in GitHub to close the repository to new issues but keep the old ones public and editable. For example, I'm still getting good information on the old closed issues, even as recently as last week: mgroth0 commented 8 days ago https://github.com/javafxports/openjdk-jfx/issues/229#issuecomment-804546080 That doesn't seem possible, though. I can find only a checkbox to enable or disable issues. John
Re: RFR: 8238650: Allow to override buildDate with SOURCE_DATE_EPOCH
On Thu, 25 Mar 2021 02:35:24 GMT, John Neffenger wrote: >>> @jgneff Could not parse `bmwiedemann` as a valid contributor. >> >> You might retry the "contributor" command with the `@` before their GitHub >> username (I'm not 100% sure that will work, though). > >> You might retry the "contributor" command with the `@` before their GitHub >> username. > > Thanks. I'll try that now. I checked and Bernhard is on the list of "OCAs > submitted prior to 2021," but not on the OpenJDK Census list, in case that > matters. > > By the way, should I be adding my own name, too? It looks as if sometimes > people do that and sometimes not. That would put me as "Author" and also on a > line with "Co-authored-by" in the commit message. > This is a simple enough change by itself, but since it is intended as the > start of a larger effort, I'd like @johanvos (or someone else he designates > from Gluon) to be a second reviewer. I jumped the gun and created the [follow-up pull request][1]. I thought it might help when reviewing this pull request to have a full picture of that larger effort. Although these two changes are part of a huge undertaking in the open-source community, it turns out that the changes required on our part are quite small. [1]: https://github.com/openjdk/jfx/pull/446 - PR: https://git.openjdk.java.net/jfx/pull/422
Re: RFR: 8264449: Enable reproducible builds with SOURCE_DATE_EPOCH
On Tue, 30 Mar 2021 16:20:21 GMT, John Neffenger wrote: > This pull request allows for reproducible builds of JavaFX on Linux, macOS, > and Windows by defining the `SOURCE_DATE_EPOCH` environment variable. For > example, the following commands create a reproducible build: > > $ export SOURCE_DATE_EPOCH=$(git log -1 --pretty=%ct) > $ bash gradlew sdk jmods javadoc > $ strip-nondeterminism -v -T $SOURCE_DATE_EPOCH build/jmods/*.jmod > > The three commands: > > 1. set the build timestamp to the date of the latest source code change, > 2. build the JavaFX SDK libraries, JMOD archives, and API documentation, and > 3. recreate the JMOD files with stable file modification times and ordering. > > The third command won't be necessary once Gradle can build the JMOD archives > or the `jmod` tool itself has the required support. For more information on > the environment variable, see the [`SOURCE_DATE_EPOCH`][1] page. For more > information on the command to recreate the JMOD files, see the > [`strip-nondeterminism`][2] repository. I'd like to propose that we allow for > reproducible builds in JavaFX 17 and consider making them the default in > JavaFX 18. > > Fixes > > There are at least four sources of non-determinism in the JavaFX builds: > > 1. Build timestamp > > The class `com.sun.javafx.runtime.VersionInfo` in the JavaFX Base module > stores the time of the build. Furthermore, for builds that don't run on the > Hudson continuous integration tool, the class adds the build time to the > system property `javafx.runtime.version`. > > 2. Modification times > > The JAR, JMOD, and ZIP archives store the modification time of each file. > > 3. File ordering > > The JAR, JMOD, and ZIP archives store their files in the order returned > by the file system. The native shared libraries also store their object files > in the order returned by the file system. Most file systems, though, do not > guarantee the order of a directory's file listing. > > 4. Build path > > The class `com.sun.javafx.css.parser.Css2Bin` in the JavaFX Graphics > module stores the absolute path of its `.css` input file in the corresponding > `.bss` output file, which is then included in the JavaFX Controls module. > > This pull request modifies the Gradle and Groovy build files to fix the first > three sources of non-determinism. A later pull request can modify the Java > files to fix the fourth. > > [1]: https://reproducible-builds.org/docs/source-date-epoch/ > [2]: https://salsa.debian.org/reproducible-builds/strip-nondeterminism I think there might be a Skara bug. The pre-submit builds on Linux, macOS, and Windows completed immediately. I think that's because the first of the two commits in this pull request includes the Java Bug ID from [another pending pull request][1], because this pull request is a continuation of that one. I can squash the two commits and force-push the changes, if that would help. [1]: https://github.com/openjdk/jfx/pull/422 - PR: https://git.openjdk.java.net/jfx/pull/446
RFR: 8264449: Enable reproducible builds with SOURCE_DATE_EPOCH
This pull request allows for reproducible builds of JavaFX on Linux, macOS, and Windows by defining the `SOURCE_DATE_EPOCH` environment variable. For example, the following commands create a reproducible build: $ export SOURCE_DATE_EPOCH=$(git log -1 --pretty=%ct) $ bash gradlew sdk jmods javadoc $ strip-nondeterminism -v -T $SOURCE_DATE_EPOCH build/jmods/*.jmod The three commands: 1. set the build timestamp to the date of the latest source code change, 2. build the JavaFX SDK libraries, JMOD archives, and API documentation, and 3. recreate the JMOD files with stable file modification times and ordering. The third command won't be necessary once Gradle can build the JMOD archives or the `jmod` tool itself has the required support. For more information on the environment variable, see the [`SOURCE_DATE_EPOCH`][1] page. For more information on the command to recreate the JMOD files, see the [`strip-nondeterminism`][2] repository. I'd like to propose that we allow for reproducible builds in JavaFX 17 and consider making them the default in JavaFX 18. Fixes There are at least four sources of non-determinism in the JavaFX builds: 1. Build timestamp The class `com.sun.javafx.runtime.VersionInfo` in the JavaFX Base module stores the time of the build. Furthermore, for builds that don't run on the Hudson continuous integration tool, the class adds the build time to the system property `javafx.runtime.version`. 2. Modification times The JAR, JMOD, and ZIP archives store the modification time of each file. 3. File ordering The JAR, JMOD, and ZIP archives store their files in the order returned by the file system. The native shared libraries also store their object files in the order returned by the file system. Most file systems, though, do not guarantee the order of a directory's file listing. 4. Build path The class `com.sun.javafx.css.parser.Css2Bin` in the JavaFX Graphics module stores the absolute path of its `.css` input file in the corresponding `.bss` output file, which is then included in the JavaFX Controls module. This pull request modifies the Gradle and Groovy build files to fix the first three sources of non-determinism. A later pull request can modify the Java files to fix the fourth. [1]: https://reproducible-builds.org/docs/source-date-epoch/ [2]: https://salsa.debian.org/reproducible-builds/strip-nondeterminism - Commit messages: - Enable reproducible builds with SOURCE_DATE_EPOCH - 8238650: Allow to override buildDate with SOURCE_DATE_EPOCH Changes: https://git.openjdk.java.net/jfx/pull/446/files Webrev: https://webrevs.openjdk.java.net/?repo=jfx=446=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8264449 Stats: 36 lines in 3 files changed: 22 ins; 10 del; 4 mod Patch: https://git.openjdk.java.net/jfx/pull/446.diff Fetch: git fetch https://git.openjdk.java.net/jfx pull/446/head:pull/446 PR: https://git.openjdk.java.net/jfx/pull/446
Re: RFR: 8258663: Fixed size TableCells are not removed from sene graph when column is removed [v2]
On Tue, 30 Mar 2021 13:27:21 GMT, Jeanette Winzenburg
wrote:
>> Marius Hanl has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> 8258663: Using VirtualFlowTestUtils in tests now instead of own solution
>> -> cleaner code
>
> modules/javafx.controls/src/test/java/test/javafx/scene/control/skin/TableRowSkinTest.java
> line 132:
>
>> 130: // We save the first table row to check it later.
>> 131: AtomicReference> tableRowRef = new
>> AtomicReference<>();
>> 132:
>
> wondering a bit about this complicated test setup .. are you aware of the
> VirtualFlowTestUtils (in test.something.infrastructure)? Using it, a test
> would shrink down to something like:
>
> @Test
> public void testRemoveColumnsFixed() {
> tableView.setFixedCellSize(20);
> tableView.getColumns().remove(0, 2);
> Toolkit.getToolkit().firePulse();
> assertEquals(tableView.getVisibleLeafColumns().size(),
> VirtualFlowTestUtils.getCell(tableView,
> 0).getChildrenUnmodifiable().size());
> }
>
> Or what am I missing?
Nice catch! I tried it out and it works! And indeed the code looks much better
now.
To be fair, I had a brief look at VirtualFlowTestUtils, as other table/cell
tests uses it. Next time I should take a closer look. :P
I pushed your suggested change.
-
PR: https://git.openjdk.java.net/jfx/pull/444
Re: RFR: 8258663: Fixed size TableCells are not removed from sene graph when column is removed [v2]
> This PR fixes an issue, where table cells are not removed from the table row > when the corresponding table column got removed. This will lead to empty > "ghost" cells laying around in the table. > This bug only occurs, when a fixed cell size is set to the table. > > I also added 3 more tests beside the mandatory test, which tests my fix. > 1 alternative test, which tests the same with no fixed cell size set. > The other 2 tests are testing the same, but the table columns are set > invisible instead. > > -> Either the removal or setVisible(false) of a column should both do the > same: Remove the corresponding cells, so that there are no empty cells. > Therefore, the additional tests make sure, that the other use cases (still) > works and won't break in future (at least, without red tests ;)). > See also: TableRowSkinBase#updateCells(boolean) Marius Hanl has updated the pull request incrementally with one additional commit since the last revision: 8258663: Using VirtualFlowTestUtils in tests now instead of own solution -> cleaner code - Changes: - all: https://git.openjdk.java.net/jfx/pull/444/files - new: https://git.openjdk.java.net/jfx/pull/444/files/acc19ee4..a2a331a2 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jfx=444=01 - incr: https://webrevs.openjdk.java.net/?repo=jfx=444=00-01 Stats: 55 lines in 1 file changed: 4 ins; 37 del; 14 mod Patch: https://git.openjdk.java.net/jfx/pull/444.diff Fetch: git fetch https://git.openjdk.java.net/jfx pull/444/head:pull/444 PR: https://git.openjdk.java.net/jfx/pull/444
Re: Proposed schedule for JavaFX 17
I just noticed a copy/paste error: > We plan to fork a jfx16 stabilization branch at RDP1. That should be a "jfx17" branch. -- Kevin On 3/30/2021 7:28 AM, Kevin Rushforth wrote: Here is the proposed schedule for JavaFX 17. RDP1: Jul 8, 2021 (aka “feature freeze”) RDP2: Jul 29, 2021 Freeze: Aug 19, 2021 GA: Sep 7, 2021 We plan to fork a jfx16 stabilization branch at RDP1. The GA date is expected to be one week ahead of JDK 17, matching what we did for 16. As was done for the previous release, the start of RDP1, the start of RDP2, and the code freeze will be 16:00 UTC on the respective dates. Please let Johan or me know if you have any questions. -- Kevin
Proposed schedule for JavaFX 17
Here is the proposed schedule for JavaFX 17. RDP1: Jul 8, 2021 (aka “feature freeze”) RDP2: Jul 29, 2021 Freeze: Aug 19, 2021 GA: Sep 7, 2021 We plan to fork a jfx16 stabilization branch at RDP1. The GA date is expected to be one week ahead of JDK 17, matching what we did for 16. As was done for the previous release, the start of RDP1, the start of RDP2, and the code freeze will be 16:00 UTC on the respective dates. Please let Johan or me know if you have any questions. -- Kevin
Re: RFR: 8258777: SkinBase: add api to un-/register invalidation-/listChange listeners [v2]
On Tue, 30 Mar 2021 12:43:41 GMT, Kevin Rushforth wrote: > > The failure on Windows is because your branch isn't up-to-date with `master`, > and is missing a recent fix to the Window build. You can either `git merge > master` (after updating your `master` branch from the upstream repo) or > ignore the error. thanks for the info - will ignore for now, the merge/rebase will happen after approval anyway :) - PR: https://git.openjdk.java.net/jfx/pull/409
Re: RFR: 8258663: Fixed size TableCells are not removed from sene graph when column is removed
On Sun, 28 Mar 2021 23:13:22 GMT, Marius Hanl
wrote:
> This PR fixes an issue, where table cells are not removed from the table row
> when the corresponding table column got removed. This will lead to empty
> "ghost" cells laying around in the table.
> This bug only occurs, when a fixed cell size is set to the table.
>
> I also added 3 more tests beside the mandatory test, which tests my fix.
> 1 alternative test, which tests the same with no fixed cell size set.
> The other 2 tests are testing the same, but the table columns are set
> invisible instead.
>
> -> Either the removal or setVisible(false) of a column should both do the
> same: Remove the corresponding cells, so that there are no empty cells.
> Therefore, the additional tests make sure, that the other use cases (still)
> works and won't break in future (at least, without red tests ;)).
> See also: TableRowSkinBase#updateCells(boolean)
modules/javafx.controls/src/test/java/test/javafx/scene/control/skin/TableRowSkinTest.java
line 132:
> 130: // We save the first table row to check it later.
> 131: AtomicReference> tableRowRef = new
> AtomicReference<>();
> 132:
wondering a bit about this complicated test setup .. are you aware of the
VirtualFlowTestUtils (in test.something.infrastructure)? Using it, a test would
shrink down to something like:
@Test
public void testRemoveColumnsFixed() {
tableView.setFixedCellSize(20);
tableView.getColumns().remove(0, 2);
Toolkit.getToolkit().firePulse();
assertEquals(tableView.getVisibleLeafColumns().size(),
VirtualFlowTestUtils.getCell(tableView,
0).getChildrenUnmodifiable().size());
}
Or what am I missing?
-
PR: https://git.openjdk.java.net/jfx/pull/444
Re: RFR: 8258777: SkinBase: add api to un-/register invalidation-/listChange listeners [v2]
On Tue, 30 Mar 2021 10:06:58 GMT, Jeanette Winzenburg wrote: > hmm ... failing checks, why? The failure on Windows is because your branch isn't up-to-date with `master`, and is missing a recent fix to the Window build. You can either `git merge master` (after updating your `master` branch from the upstream repo) or ignore the error. - PR: https://git.openjdk.java.net/jfx/pull/409
Re: RFR: 8258663: Fixed size TableCells are not removed from sene graph when column is removed
On Mon, 29 Mar 2021 12:35:23 GMT, Marius Hanl
wrote:
>> modules/javafx.controls/src/main/java/javafx/scene/control/skin/TableRowSkinBase.java
>> line 556:
>>
>>> 554: }
>>> 555: getChildren().removeAll(toRemove);
>>> 556: } else if (resetChildren || cellsEmpty) {
>>
>> just curious : any idea why fixedCellSize was special-cased here? Not to
>> clean them up sounds (and is) very much wrong, so there must have been a
>> reason?
>
> The '!fixedCellSizeEnabled' check is not needed, as we already check for
> fixedCellSizeEnabled in the main if condition.
> `if (fixedCellSizeEnabled) {...}`
>
> So before, it was like:
> `if (fixedCellSizeEnabled) {...} `
> `else if (!fixedCellSizeEnabled && (resetChildren || cellsEmpty)) {...}`
>
> So we only execute the else condition, if fixedCellSizeEnabled is not true ->
> !fixedCellSizeEnabled. -> The check is not necessary, as fixedCellSizeEnabled
> must be false, when we come to the else condition.
> The variable fixedCellSizeEnabled is also a primitive boolean, so it can't be
> null, making this check obsolete.
>
> Edit: If you mean the special handling for fixed cell size in general, I have
> no idea. This was added in a commit from Jonathan Giles stating, that he
> fixed a performance problem with fixed cell size. So maybe instead of
> resetting all the cells, only the affected are removed/added by this, leading
> to a performance boost?
yeah, was curious about the stuff in your "edit" paragraph. And thanks for the
explanation of the if-block, didn't see the whole .. :)
-
PR: https://git.openjdk.java.net/jfx/pull/444
Re: RFR: 8255572: Axis does not compute preferred height properly when autoRanging is off [v6]
On Mon, 29 Mar 2021 22:19:08 GMT, Jonathan Vusich wrote: > > > @kleopatra @kevinrushforth Are the system tests that I have provided > sufficient for this PR to move forward or do they need to be rewritten as > unit tests? good question :) Personally, I would also add a unit test (even though it requires that hacky bit), it's more likely to be "seen". But also fine as-is, that is having system tests, IMO. Didn't run them, though .. *cough - PR: https://git.openjdk.java.net/jfx/pull/342
Re: RFR: 8258777: SkinBase: add api to un-/register invalidation-/listChange listeners [v4]
On Mon, 29 Mar 2021 17:17:30 GMT, Nir Lisker wrote:
>> Jeanette Winzenburg has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> fixed trailing whitespace
>
> modules/javafx.controls/src/main/java/javafx/scene/control/SkinBase.java line
> 246:
>
>> 244: /**
>> 245: * Registers an operation to perform when the given {@code
>> Observable} sends an invalidation event.
>> 246: * Does nothing if observable or operation is {@code null}.
>
> I would write "Does nothing if either {@code observable} or {@code operation}
> are {@code null}"
Done. Also changed leading upper-case of observable in first sentence to
lower-case - for consistency because I don't see a difference between both,
blind me? ;)
> modules/javafx.controls/src/main/java/javafx/scene/control/SkinBase.java line
> 265:
>
>> 263: * Unregisters all operations that have been registered using
>> 264: * {@link #registerInvalidationListener(Observable, Consumer)}
>> 265: * for the given observable.
>
> If the parameter can be `null`, mention what happens like in
> `registerInvalidationListener`.
done
> modules/javafx.controls/src/main/java/javafx/scene/control/SkinBase.java line
> 270:
>
>> 268: * may be {@code null}
>> 269: * @return a composed consumer that performs all removed operations
>> or
>> 270: * {@code null} if none has been registered or the observable is
>> {@null}
>
> * Comma before the first "or"
> * "none *have* been"
thanks :) Done.
-
PR: https://git.openjdk.java.net/jfx/pull/409
Re: RFR: 8258777: SkinBase: add api to un-/register invalidation-/listChange listeners [v2]
On Thu, 25 Mar 2021 14:07:24 GMT, Jeanette Winzenburg wrote: >> I see. I recommend that they be improved in this PR. I don't know if this >> will need to be part of the CSR, though. > > @nlisker and @Kevin so we agree, thanks :) > > my plan: > > - will work on the exact doc along the lines of Nir's suggestion for the > un-/registerInvalidationListener methods > - once that's basically agreed upon, will c the spec (with adjustments) to > the methods for the other listener types > - at a last step, create the csr draft (that would be a patch for the > changeListener methods and simply added spec for the new methods, I assume?) hmm ... failing checks, why? - PR: https://git.openjdk.java.net/jfx/pull/409
Re: RFR: 8258777: SkinBase: add api to un-/register invalidation-/listChange listeners [v5]
> Changes in Lambda..Handler: > - added api and implemenation to support invalidation and listChange > listeners in the same way as changeListeners > - added java doc > - added tests > > Changes in SkinBase > - added api (and implementation delegating to the handler) > - copied java doc from the change listener un/register methods > - added tests to verify that the new (and old) api is indeed delegating to > the handler > > Note that the null handling is slightly extended: all methods now can handle > both null consumers (as before) and null observables (new) - this allows > simplified code on rewiring "path" properties (see reference example in the > issue) Jeanette Winzenburg has updated the pull request incrementally with one additional commit since the last revision: xxInvalidationListener: changed doc as per review - Changes: - all: https://git.openjdk.java.net/jfx/pull/409/files - new: https://git.openjdk.java.net/jfx/pull/409/files/68d70c6b..cf3a0228 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jfx=409=04 - incr: https://webrevs.openjdk.java.net/?repo=jfx=409=03-04 Stats: 5 lines in 1 file changed: 0 ins; 0 del; 5 mod Patch: https://git.openjdk.java.net/jfx/pull/409.diff Fetch: git fetch https://git.openjdk.java.net/jfx pull/409/head:pull/409 PR: https://git.openjdk.java.net/jfx/pull/409
