Re: RFR: 8255940: localStorage is null after window.close() [v2]
On Fri, 28 Jan 2022 00:11:40 GMT, Kevin Rushforth wrote: >> Jay Bhaskar 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 two additional >> commits since the last revision: >> >> - Merge branch 'openjdk:master' into PRLocalstorage >> - Window.close(), Fix for localstoarge > > modules/javafx.web/src/test/java/test/javafx/scene/web/LocalStorageTest.java > line 2: > >> 1: /* >> 2: * Copyright (c) 2011, 2020, Oracle and/or its affiliates. All rights >> reserved. > > Copyright should be a single year (2022) done > modules/javafx.web/src/test/java/test/javafx/scene/web/LocalStorageTest.java > line 48: > >> 46: final WebEngine webEngine = getEngine(); >> 47: webEngine.setJavaScriptEnabled(true); >> 48: webEngine.setUserDataDirectory(new File("/tmp/java-store")); > > You should not hard-code /tmp/. You can either use a local subdirectory in > the build dir (which some other tests do), or else you will need to use > something like `Files.createTempDirectory("webstorage")`. If you use the > latter, then you will need to worry about how to clean up after the test, so > the former seems better. i think /tmp/java-store is ok, as it would not require cleanup > modules/javafx.web/src/test/java/test/javafx/scene/web/LocalStorageTest.java > line 70: > >> 68: WebView view = getView(); >> 69: view.getEngine().executeScript("test_local_storage_set();"); >> 70: String s = (String) >> view.getEngine().executeScript("document.getElementById('key').innerText;"); > > This will work, but it might be cleaner to add a JavaScript `getLocalStorage` > method so you don't have to get it from a DOM element. The method webEngine.executeScript("localStorage;") returns JSObject and in to get data members we need to call js.getMember(...) , getMember is not available in our code base, so i used tring s = (String) view.getEngine().executeScript("document.getElementById('key').innerText;"); > modules/javafx.web/src/test/java/test/javafx/scene/web/LocalStorageTest.java > line 80: > >> 78: submit(() -> { >> 79: WebView view = getView(); >> 80: view.getEngine().executeScript("delete_items();"); > > You need to set some items first before deleting them if you want it to be an > effective test of this case. The test runs after testLocalStorageSet , so there would be items in localstorage - PR: https://git.openjdk.java.net/jfx/pull/703
Re: RFR: 8273336: Clicking a selected cell from a group of selected cells in a TableView clears the selected items list but remains selected
On Fri, 7 Jan 2022 19:36:45 GMT, Jose Pereda wrote: > This PR adds a predicate to TableView and TreeTableView selection models > order to remove rows from the selection only when there are no selected cells > in that given row, when cell selection is enabled. > > Two tests have been added as well, that fail without this PR and pass with it. Why not use IntPredicate? before ``` java public static void updateSelectedIndices(MultipleSelectionModelBase sm, ListChangeListener.Change> c, Predicate removeRowFilter) { after ``` java public static void updateSelectedIndices(MultipleSelectionModelBase sm, ListChangeListener.Change> c, IntPredicate removeRowFilter) { before ``` java .map(TablePositionBase::getRow) after ``` java .mapToInt(TablePositionBase::getRow) - PR: https://git.openjdk.java.net/jfx/pull/709
Re: RFR: 8255940: localStorage is null after window.close() [v2]
> Issue: The current implementation of DOMWindow ::localStorage(..) returns > null pointer in case of page is being closed. > Fix: It should not return nullptr , as per the [w3c web storage > spec](https://www.w3.org/TR/2016/REC-webstorage-20160419/) > "User agents should expire data from the local storage areas only for > security reasons or when requested to do so by the user. User agents should > always avoid deleting data while a script that could access that data is > running." Jay Bhaskar 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 two additional commits since the last revision: - Merge branch 'openjdk:master' into PRLocalstorage - Window.close(), Fix for localstoarge - Changes: - all: https://git.openjdk.java.net/jfx/pull/703/files - new: https://git.openjdk.java.net/jfx/pull/703/files/12f97feb..73299577 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jfx&pr=703&range=01 - incr: https://webrevs.openjdk.java.net/?repo=jfx&pr=703&range=00-01 Stats: 416909 lines in 6905 files changed: 234954 ins; 135768 del; 46187 mod Patch: https://git.openjdk.java.net/jfx/pull/703.diff Fetch: git fetch https://git.openjdk.java.net/jfx pull/703/head:pull/703 PR: https://git.openjdk.java.net/jfx/pull/703
Re: [jfx18] RFR: 8279345: Realign class docs of LightBase and subclasses [v5]
On Fri, 4 Feb 2022 18:26:57 GMT, Nir Lisker wrote: >> Now that the standard concrete light types were added, there is an >> opportunity to rearrange and rewrite some of the class docs. Here is a >> summary of the changes: >> >> * Moved the explanations of attenuation and direction up to `LightBase` >> since different light types share characteristics. `LightBase` now contains >> a summary of its subtypes and all the explanations of the >> properties/characteristics of the lights divided into sections: Color, >> Scope, Direction, Attenuation. >> * Each light type links to the relevant section in `LightBase` when it >> mentioned the properties it has. >> * Added examples of real-world applications for each light type. >> * Consolidated the writing style for all the subclasses. > > Nir Lisker has updated the pull request incrementally with one additional > commit since the last revision: > > Changed description of ambient light LGTM. I like your updated AmbientLight description. - Marked as reviewed by kcr (Lead). PR: https://git.openjdk.java.net/jfx/pull/717
Re: RFR: 8277309: Add support for H.265/HEVC to HTTP Live Streaming
On Fri, 4 Feb 2022 11:24:48 GMT, Alexander Matveev wrote: > - Added support for fragmented MP4 with HEVC/H.265, H.264 and AAC. > - Added support for elementary AAC streams without any container for audio > only streams. > - Added "aacparse" plugin from GStreamer. Required on Linux, since decoder > cannot handle AAC elementary streams directly. DirectShow decoder works > without it. > - DirectShow H.264 decoder on Windows and H.265/H.264 decoder on Linux will > be reloaded when fMP4 stream changes resolution. Dynamic format change did > not worked for these streams on Windows and Linux. modules/javafx.media/src/main/java/com/sun/media/jfxmedia/locator/HLSConnectionHolder.java I noticed that in this change, ArrayList is rewritten as ArrayList<>. The BitSet rewrite needs some thought, so it might be a good idea to make it a separate issue. In the file I pointed out, it is used in many more places than the one I pointed out. - PR: https://git.openjdk.java.net/jfx/pull/726
Re: [jfx18] RFR: 8279345: Realign class docs of LightBase and subclasses [v5]
On Thu, 27 Jan 2022 19:01:32 GMT, Kevin Rushforth wrote: >> I think the description should focus on the meaning of the respective term >> in the lighting equation, and not on a non-technical analogy. In this case, >> the analogy is a bit misleading on several aspects, including the fact that >> ambient lighting is not dependent on an area being enclosed. Here's a >> suggestion: >> >> Ambient lights add a constant term to the amount of light reflected by each >> point on >> the surface of an object, thereby increasing the brightness of the object >> uniformly and >> independently of the orientation of its surfaces. It is often used to >> represent the base >> amount of illumination in a scene. > > I think the key aspects of an Ambient light are that: the light seems to come > from all directions (and thus has no position or direction), and that it > illuminates objects independently of the position or orientation of the > object. > > I don't have a problem with also giving a real-world analogy if one can be > found that makes it easier to understand without causing confusion. I tried to unify some of the points that were brought up. See if you would like more changes. - PR: https://git.openjdk.java.net/jfx/pull/717
Re: [jfx18] RFR: 8279345: Realign class docs of LightBase and subclasses [v5]
> Now that the standard concrete light types were added, there is an > opportunity to rearrange and rewrite some of the class docs. Here is a > summary of the changes: > > * Moved the explanations of attenuation and direction up to `LightBase` since > different light types share characteristics. `LightBase` now contains a > summary of its subtypes and all the explanations of the > properties/characteristics of the lights divided into sections: Color, Scope, > Direction, Attenuation. > * Each light type links to the relevant section in `LightBase` when it > mentioned the properties it has. > * Added examples of real-world applications for each light type. > * Consolidated the writing style for all the subclasses. Nir Lisker has updated the pull request incrementally with one additional commit since the last revision: Changed description of ambient light - Changes: - all: https://git.openjdk.java.net/jfx/pull/717/files - new: https://git.openjdk.java.net/jfx/pull/717/files/cae874d6..987a6165 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jfx&pr=717&range=04 - incr: https://webrevs.openjdk.java.net/?repo=jfx&pr=717&range=03-04 Stats: 6 lines in 1 file changed: 2 ins; 0 del; 4 mod Patch: https://git.openjdk.java.net/jfx/pull/717.diff Fetch: git fetch https://git.openjdk.java.net/jfx pull/717/head:pull/717 PR: https://git.openjdk.java.net/jfx/pull/717
Re: RFR: 8277309: Add support for H.265/HEVC to HTTP Live Streaming
On Fri, 4 Feb 2022 17:19:36 GMT, yosbits wrote: > Why not use BitSet instead of ArrayList? Can you be more specific? The only use of `ArrayList` that I see in the patch is in code that uses a few already-existing lists. Changing that to some other data structure would be an unrelated change that seems out of scope for this fix. - PR: https://git.openjdk.java.net/jfx/pull/726
Re: RFR: 8273336: Clicking a selected cell from a group of selected cells in a TableView clears the selected items list but remains selected
On Fri, 7 Jan 2022 19:36:45 GMT, Jose Pereda wrote: > This PR adds a predicate to TableView and TreeTableView selection models > order to remove rows from the selection only when there are no selected cells > in that given row, when cell selection is enabled. > > Two tests have been added as well, that fail without this PR and pass with it. modules/javafx.controls/src/main/java/javafx/scene/control/ControlUtils.java line 172: > 170: .map(TablePositionBase::getRow) > 171: .filter(removeRowFilter) > 172: .distinct() Maybe `distinct` should be applied before `filter`. This can cut down the number of times the predicate is invoked (which iterates over all selected cells, so it may be a performance issue for large selections). - PR: https://git.openjdk.java.net/jfx/pull/709
Re: RFR: 8277309: Add support for H.265/HEVC to HTTP Live Streaming
On Fri, 4 Feb 2022 11:24:48 GMT, Alexander Matveev wrote: > - Added support for fragmented MP4 with HEVC/H.265, H.264 and AAC. > - Added support for elementary AAC streams without any container for audio > only streams. > - Added "aacparse" plugin from GStreamer. Required on Linux, since decoder > cannot handle AAC elementary streams directly. DirectShow decoder works > without it. > - DirectShow H.264 decoder on Windows and H.265/H.264 decoder on Linux will > be reloaded when fMP4 stream changes resolution. Dynamic format change did > not worked for these streams on Windows and Linux. Why not use BitSet instead of ArrayList? - PR: https://git.openjdk.java.net/jfx/pull/726
Integrated: 8278980: Update WebKit to 613.1
On Wed, 2 Feb 2022 07:34:55 GMT, Ambarish Rapte wrote: > Update JavaFX WebKit to GTK WebKit 2.34 (613.1). > > Verified the updated version build, tests run and sanity testing. > This does not cause any issues except a unit test failure > `IrresponsiveScriptTest`. > It is recorded and ignored using > [JDK-8280421](https://bugs.openjdk.java.net/browse/JDK-8280421) This pull request has now been integrated. Changeset: 6f28d912 Author:Ambarish Rapte URL: https://git.openjdk.java.net/jfx/commit/6f28d912024495278c4c35ab054bc2aab480b3e4 Stats: 412962 lines in 6737 files changed: 231442 ins; 135635 del; 45885 mod 8278980: Update WebKit to 613.1 Co-authored-by: Ajit Ghaisas Co-authored-by: Jay Bhaskar Co-authored-by: Kevin Rushforth Reviewed-by: kcr, jvos, aghaisas - PR: https://git.openjdk.java.net/jfx/pull/723
RFR: 8277309: Add support for H.265/HEVC to HTTP Live Streaming
- Added support for fragmented MP4 with HEVC/H.265, H.264 and AAC. - Added support for elementary AAC streams without any container for audio only streams. - Added "aacparse" plugin from GStreamer. Required on Linux, since decoder cannot handle AAC elementary streams directly. DirectShow decoder works without it. - DirectShow H.264 decoder on Windows and H.265/H.264 decoder on Linux will be reloaded when fMP4 stream changes resolution. Dynamic format change did not worked for these streams on Windows and Linux. - Commit messages: - 8277309: Add support for H.265/HEVC to HTTP Live Streaming Changes: https://git.openjdk.java.net/jfx/pull/726/files Webrev: https://webrevs.openjdk.java.net/?repo=jfx&pr=726&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8277309 Stats: 2637 lines in 27 files changed: 2343 ins; 126 del; 168 mod Patch: https://git.openjdk.java.net/jfx/pull/726.diff Fetch: git fetch https://git.openjdk.java.net/jfx pull/726/head:pull/726 PR: https://git.openjdk.java.net/jfx/pull/726
Re: RFR: 8278980: Update WebKit to 613.1 [v2]
On Wed, 2 Feb 2022 15:29:51 GMT, Ambarish Rapte wrote: >> Update JavaFX WebKit to GTK WebKit 2.34 (613.1). >> >> Verified the updated version build, tests run and sanity testing. >> This does not cause any issues except a unit test failure >> `IrresponsiveScriptTest`. >> It is recorded and ignored using >> [JDK-8280421](https://bugs.openjdk.java.net/browse/JDK-8280421) > > Ambarish Rapte has updated the pull request incrementally with one additional > commit since the last revision: > > Restore commits that were mistakenly reverted. > > 8279013: ES2Pipeline fails to detect AMD vega20 graphics card > 8277853: With Touch enabled devices scrollbar disappears and the table is > scrolled to the beginning > 8277122: SplitPane divider drag can hang the layout Marked as reviewed by aghaisas (Reviewer). I have tested on macOS and Windows. No new issue observed. - PR: https://git.openjdk.java.net/jfx/pull/723
Re: RFR: 8278980: Update WebKit to 613.1 [v2]
On Wed, 2 Feb 2022 15:29:51 GMT, Ambarish Rapte wrote: >> Update JavaFX WebKit to GTK WebKit 2.34 (613.1). >> >> Verified the updated version build, tests run and sanity testing. >> This does not cause any issues except a unit test failure >> `IrresponsiveScriptTest`. >> It is recorded and ignored using >> [JDK-8280421](https://bugs.openjdk.java.net/browse/JDK-8280421) > > Ambarish Rapte has updated the pull request incrementally with one additional > commit since the last revision: > > Restore commits that were mistakenly reverted. > > 8279013: ES2Pipeline fails to detect AMD vega20 graphics card > 8277853: With Touch enabled devices scrollbar disappears and the table is > scrolled to the beginning > 8277122: SplitPane divider drag can hang the layout builds on all platforms, basic tests are ok. We should do an EA release soon after this is merged, so we can have more testers. - Marked as reviewed by jvos (Reviewer). PR: https://git.openjdk.java.net/jfx/pull/723