Re: RFR: 8272118: ListViewSkin et al: must not cancel edit on scrolling

2021-12-01 Thread Kevin Rushforth
On Thu, 25 Nov 2021 15:46:01 GMT, Jeanette Winzenburg  
wrote:

> Issue was that mouse pressed on the scrollbars of all virtualized controls 
> cancelled the edit. That's inconsistent with other scroll triggers 
> (mouseWheel, programmatic). Fixed by removing the cancel.
> 
> Added tests that failed/passed before/after the fix. Also added tests that 
> passed both before/after to guarantee that required functionality of the 
> mouse pressed (= requesting focus on the control if needed) is still working.

Looks good. Thanks for adding all the tests!

modules/javafx.controls/src/test/java/test/javafx/scene/control/skin/EditAndScrollTest.java
 line 408:

> 406: 
> 407: 
> 408: //- Utility methods (TODO: move into infrastructure)

Can you file a follow-up issue for this?

-

Marked as reviewed by kcr (Lead).

PR: https://git.openjdk.java.net/jfx/pull/682


RFR: 8278021: Fix warnings in macOS glass native code and treat warnings as errors

2021-12-01 Thread Martin Fox
Turning on warnings-as-errors for the macOS glass native code. Deprecated 
declarations are excluded and still appear as warnings.

In the code that tries to locate the application's dock icon there were three 
instances where `NO` was being passed into a method that required a pointer to 
a `BOOL`, not a `BOOL`. I suspect the intent was to check that the path pointed 
to an existing file but not a directory. Since JavaFX has gone this long 
without screening out directories correctly I decided not to fix that behavior 
except at the very end.

The only other changes of note are sending some NSNotification objects to 
delegate API's that require them even though we know they're ignored on the 
other side. It was the easiest way to get rid of the warning.

-

Commit messages:
 - Mac Glass treats errors as warnings except for deprecated declarations

Changes: https://git.openjdk.java.net/jfx/pull/687/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jfx=687=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8278021
  Stats: 20 lines in 8 files changed: 12 ins; 0 del; 8 mod
  Patch: https://git.openjdk.java.net/jfx/pull/687.diff
  Fetch: git fetch https://git.openjdk.java.net/jfx pull/687/head:pull/687

PR: https://git.openjdk.java.net/jfx/pull/687


Integrated: 8277457: AccessControlException: access denied ("java.net.NetPermission" "getCookieHandler")

2021-12-01 Thread Kevin Rushforth
On Wed, 24 Nov 2021 23:05:05 GMT, Kevin Rushforth  wrote:

> As indicated in the bug report, WebView needs to call to 
> `CookieManager::getDefault` within a `doPrivileged` block so that it will 
> work when a security manager is enabled. There are two calls in 
> `com.sun.webkit.network.CookieJar` that are not wrapped in a `doPrivileged`.
> 
> I added a manual test (since it requires loading a page over http/https which 
> we can't do in our automated tests) based on the test program that was 
> attached to JBS.

This pull request has now been integrated.

Changeset: 5bd72a7c
Author:Kevin Rushforth 
URL:   
https://git.openjdk.java.net/jfx/commit/5bd72a7c991d5182f4fd2cff67174cf7fa3fde85
Stats: 101 lines in 2 files changed: 99 ins; 0 del; 2 mod

8277457: AccessControlException: access denied ("java.net.NetPermission" 
"getCookieHandler")

Reviewed-by: jpereda, aghaisas

-

PR: https://git.openjdk.java.net/jfx/pull/681


Re: RFR: 8277457: AccessControlException: access denied ("java.net.NetPermission" "getCookieHandler") [v2]

2021-12-01 Thread Ajit Ghaisas
On Thu, 25 Nov 2021 00:29:36 GMT, Kevin Rushforth  wrote:

>> As indicated in the bug report, WebView needs to call to 
>> `CookieManager::getDefault` within a `doPrivileged` block so that it will 
>> work when a security manager is enabled. There are two calls in 
>> `com.sun.webkit.network.CookieJar` that are not wrapped in a `doPrivileged`.
>> 
>> I added a manual test (since it requires loading a page over http/https 
>> which we can't do in our automated tests) based on the test program that was 
>> attached to JBS.
>
> Kevin Rushforth has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   Address review feedback: use method reference instead of lambda

Marked as reviewed by aghaisas (Reviewer).

-

PR: https://git.openjdk.java.net/jfx/pull/681


Re: RFR: 8276313: ScrollPane scroll delta incorrectly depends on content height [v2]

2021-12-01 Thread Michael Strauß
On Wed, 1 Dec 2021 09:45:40 GMT, Ajit Ghaisas  wrote:

>> Michael Strauß has updated the pull request incrementally with one 
>> additional commit since the last revision:
>> 
>>   Handle division by zero
>
> modules/javafx.controls/src/main/java/javafx/scene/control/skin/ScrollPaneSkin.java
>  line 896:
> 
>> 894: double vPixelValue;
>> 895: if (nodeHeight > 0.0) {
>> 896: vPixelValue = vRange / (nodeHeight - contentHeight);
> 
> This may result in divide by 0.

Good catch! I've also fixed a few similar issues in other places.

-

PR: https://git.openjdk.java.net/jfx/pull/660


Re: RFR: 8276313: ScrollPane scroll delta incorrectly depends on content height [v2]

2021-12-01 Thread Michael Strauß
> This PR fixes an issue where the scroll delta of ScrollPane incorrectly 
> depends on the size of its content.
> This leads to extremely slow scrolling when the content is only slightly 
> larger than the ScrollPane.

Michael Strauß has updated the pull request incrementally with one additional 
commit since the last revision:

  Handle division by zero

-

Changes:
  - all: https://git.openjdk.java.net/jfx/pull/660/files
  - new: https://git.openjdk.java.net/jfx/pull/660/files/1605eec4..147858d7

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jfx=660=01
 - incr: https://webrevs.openjdk.java.net/?repo=jfx=660=00-01

  Stats: 19 lines in 1 file changed: 4 ins; 11 del; 4 mod
  Patch: https://git.openjdk.java.net/jfx/pull/660.diff
  Fetch: git fetch https://git.openjdk.java.net/jfx pull/660/head:pull/660

PR: https://git.openjdk.java.net/jfx/pull/660


Integrated: 8277475: Update JDK_DOCS property to point to JDK 17 docs

2021-12-01 Thread Kevin Rushforth
On Tue, 30 Nov 2021 15:58:17 GMT, Kevin Rushforth  wrote:

> We are still pointing to the JDK 12 API docs when building the JavaFX docs. 
> This PR updates the build to point to the JDK 17 API docs.

This pull request has now been integrated.

Changeset: 3d572135
Author:Kevin Rushforth 
URL:   
https://git.openjdk.java.net/jfx/commit/3d572135d44b41945b87c297b33b3527814b7e51
Stats: 4 lines in 2 files changed: 0 ins; 0 del; 4 mod

8277475: Update JDK_DOCS property to point to JDK 17 docs

Reviewed-by: jvos

-

PR: https://git.openjdk.java.net/jfx/pull/686


Re: RFR: 8272118: ListViewSkin et al: must not cancel edit on scrolling

2021-12-01 Thread Ajit Ghaisas
On Thu, 25 Nov 2021 15:46:01 GMT, Jeanette Winzenburg  
wrote:

> Issue was that mouse pressed on the scrollbars of all virtualized controls 
> cancelled the edit. That's inconsistent with other scroll triggers 
> (mouseWheel, programmatic). Fixed by removing the cancel.
> 
> Added tests that failed/passed before/after the fix. Also added tests that 
> passed both before/after to guarantee that required functionality of the 
> mouse pressed (= requesting focus on the control if needed) is still working.

Looks good to me.

-

Marked as reviewed by aghaisas (Reviewer).

PR: https://git.openjdk.java.net/jfx/pull/682


Re: RFR: 8276313: ScrollPane scroll delta incorrectly depends on content height

2021-12-01 Thread Ajit Ghaisas
On Tue, 2 Nov 2021 10:49:45 GMT, Michael Strauß  wrote:

> This PR fixes an issue where the scroll delta of ScrollPane incorrectly 
> depends on the size of its content.
> This leads to extremely slow scrolling when the content is only slightly 
> larger than the ScrollPane.

modules/javafx.controls/src/main/java/javafx/scene/control/skin/ScrollPaneSkin.java
 line 896:

> 894: double vPixelValue;
> 895: if (nodeHeight > 0.0) {
> 896: vPixelValue = vRange / (nodeHeight - contentHeight);

This may result in divide by 0.

modules/javafx.controls/src/main/java/javafx/scene/control/skin/ScrollPaneSkin.java
 line 928:

> 926: double hPixelValue;
> 927: if (nodeWidth > 0.0) {
> 928: hPixelValue = hRange / (nodeWidth - contentWidth);

This may result in divide by 0.

-

PR: https://git.openjdk.java.net/jfx/pull/660