Re: [Rev 02] RFR: 8227425: Add support for e-paper displays on i.MX6 devices

[Kevin Rushforth]

On Fri, 17 Apr 2020 19:11:19 GMT, John Neffenger 
 wrote:

>> This pull request adds support for e-paper displays with the i.MX 6 Series 
>> of Applications Processors and implements
>> [Issue #521](https://github.com/javafxports/openjdk-jfx/issues/521) in the 
>> obsolete *javafxports/openjdk-jfx*
>> repository. Some of the changes were made to support the new device models, 
>> while others are minor changes to the
>> existing support in JavaFX 13.  The following changes were made to support 
>> the new device models.
>> * Work around problems on the Kobo Glo HD Model N437.
>> 
>> Ignore the error ENOTTY (25), "Inappropriate ioctl for device," when 
>> setting the waveform modes. The IOCTL call is
>> ignored by the driver on the Kobo Glo HD where the error occurs, anyway.
>> 
>> Use the visible y-resolution (`yres`), not the virtual y-resolution 
>> (`yres_virtual`), when calculating the capacity of
>> the off-screen byte buffer and the length of the frame buffer mapping. 
>> The virtual y-resolution reported by the frame
>> buffer device driver can be an incorrect value.
>> 
>> * Work around problems on the Kobo Clara HD Model N249.
>> 
>> The Kobo Clara HD requires the native screen width to be set to the 
>> visible x-resolution (`xres`), instead than the
>> normal virtual x-resolution (`xres_virtual`), when using an unrotated 
>> and uninverted 8-bit grayscale frame buffer. The
>> work-around is provided through a new Boolean system property called 
>> *monocle.epd.fixWidthY8UR*.
>> 
>> The following changes were made to the existing code that supports e-paper 
>> displays in JavaFX 13.
>> 
>> * Use the correct constant for the number of bytes per pixel.
>> 
>> Use the number of bytes per pixel (`Integer.BYTES`), not the number of 
>> bits per pixel (`Integer.SIZE`), when
>> calculating the capacity of the 32-bit off-screen byte buffer.
>> 
>> * Do not round the luma value to the nearest integer.
>> 
>> Use the value of luma rounded toward zero automatically by Java when 
>> converting a `float` to an `int` instead of
>> rounding to the nearest integer. The additional rounding operation can 
>> decrease performance anywhere from zero to 10
>> percent and doesn't seem worth it for a display with just 16 levels of 
>> gray.
>> 
>> * Change camel case of system property *y8inverted*.
>> 
>> Change the camel case of the system property *monocle.epd.y8inverted* to 
>> the form *monocle.epd.Y8Inverted* so that it
>> is consistent with the other system properties containing Y1, Y4, and Y8 
>> in their names.
>> 
>> * Improve error handling when mapping the frame buffer.
>> 
>> Log the mapping and unmapping of the frame buffer. Log any errors that 
>> occur on either of the system calls. Handle a
>> `null` return value from `EPDFrameBuffer.getMappedBuffer`.
>> 
>> * Replace non-ASCII characters with their ASCII equivalent.
>> 
>> Replace non-ASCII characters in comments and log messages as follows:
>> 
>> * "×" with "x" for display resolution and color depth,
>> * "×" with "*" for multiplication,
>> * "→" with "to" for transition, and
>> * "°C" with "degrees Celsius" for temperature.
>> 
>> * Add descriptions to Monocle EPD system properties.
>> 
>> Add Javadoc comments to each constant that defines a Monocle EPD system 
>> property.
>> 
>> * Rename `IntStructure.getInteger` to `IntStructure.get`.
>> 
>> Rename the methods `getInteger` and `setInteger` in `IntStructure` to 
>> avoid confusion with the Java method
>> `Integer.getInteger`, which gets the integer value of a system property.
>> 
>> Below are some of the more interesting test results.
>> 
>> * The Kobo Glo HD and Kobo Clara HD implement a true GC4 waveform mode that 
>> displays only pixels with the four gray
>>   values `0x00`, `0x55`, `0xAA`, and `0xFF`. On the older Kobo Touch models, 
>> the GC4 waveform mode is the same as GC16.
>> 
>> * When the *forceMonochrome* update flag is enabled, the Kobo Clara HD uses 
>> a zero-percent threshold that displays black
>>   for pixels with the value `0x00` and white for all other values. The other 
>> models use a 50-percent threshold that
>>   displays black for values `0x7F` or less and white for values `0x80` or 
>> greater.
>> 
>> * The OpenJDK 11 package in Ubuntu 18.04 LTS runs twice as fast as the 
>> AdoptOpenJDK build of OpenJDK 13 for some of the
>>   tests. The speed difference is greatest for the tests that require pixel 
>> conversion into an 8-bit or 16-bit frame
>>   buffer. I plan to investigate the cause of the performance difference.
>> 
>> * In general, the faster processor and memory bus of the newer models does 
>> not fully compensate for the threefold
>>   increase in the number of pixels in their displays. The table below shows 
>> the animation speed of each model in
>>   milliseconds per frame and frames per second.
>> 
>> | Product Name  | Speed (ms) | Rate (fps) |
>> 

Re: [Rev 01] RFR: 8243255: Font size is large in JavaFX app with enabled Monocle on Raspberry Pi

[Kevin Rushforth]

On Mon, 27 Apr 2020 11:09:51 GMT, Alexander Scherbatiy  
wrote:

>> See the detailed issue description on: 
>> http://mail.openjdk.java.net/pipermail/openjfx-dev/2020-April/025975.html
>> 
>> The fix 8236448 https://github.com/openjdk/jfx/pull/75 changes
>> [MonocleApplication.staticScreen_getScreens()](https://github.com/openjdk/jfx/pull/75/files#diff-b66ff7fe72c6c5cd26003572ca901bfdL228)
>> method code from
>>> ns.getDPI(), ns.getDPI(), ns.getScale(), ns.getScale(), ns.getScale(), 
>>> ns.getScale()
>> 
>>  to
>>  > ns.getWidth(), ns.getHeight(), 1.f, 1.f, ns.getScale(), ns.getScale()"
>> 
>> so the font size is not properly calculated based on the given DPI value.
>> 
>> I left the platformScaleX and platformScaleY as 1.f because I do not know 
>> how it affects Android/Dalvik platform. On
>> Raspberry Pi where I run JavaFX code with Monocle the DispmanScreen is used 
>> which have fixed scale 1.0 value so the app
>> works in the same way.
>
> Alexander Scherbatiy has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   Search javafx.platform.properties in jfx-runtime/lib directory

Marked as reviewed by kcr (Lead).

-

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

Re: [Rev 01] RFR: 8243255: Font size is large in JavaFX app with enabled Monocle on Raspberry Pi

[Kevin Rushforth]

On Tue, 28 Apr 2020 23:17:48 GMT, Kevin Rushforth  wrote:

>> Alexander Scherbatiy has updated the pull request incrementally with one 
>> additional commit since the last revision:
>> 
>>   Search javafx.platform.properties in jfx-runtime/lib directory
>
> Marked as reviewed by kcr (Lead).

Pending review by @johanvos

> modules/javafx.base/src/main/java/com/sun/javafx/PlatformUtil.java line 241:
> 
>> 240: s.lastIndexOf('/'), s.lastIndexOf('\\'));
>> 241: return new File(new URL(s.substring(0, lastIndexOfSlash + 
>> 1)).getPath());
>> 242: } catch (MalformedURLException e) {
> 
> The previous code looks like a hold-over from JDK 8, where jfxrt.jar was in 
> `lib/ext`. It also assumes that the JavaFX
> runtime is being loaded from a jar URL, which isn't necessarily the case. 
> Probably the only reason it hasn't caused
> problems before now is that it is only used to locate 
> `javafx.platform.properties`. Worth noting is that we won't get
> here in the case JavaFX is jlinked into the Java runtime, although in that 
> case, the fallback method of locating it
> relative to the JDK should be used.  I'll take a closer look this specific 
> change, but I think it should be OK.  I'll
> defer the review as a whole to Johan.

I can confirm that this part of the fix is correct. The change to DPI looks 
correct to me, too.

-

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

Re: [Rev 03] RFR: 8202296: Monocle MouseInput doesn't send keyboard modifiers in events.

[Kevin Rushforth]

On Wed, 15 Apr 2020 08:28:22 GMT, Tom Schindl  wrote:

>> Extract keystate and add to the existing modifier mask, to support eg
>> multi-select
>> 
>> https://bugs.openjdk.java.net/browse/JDK-8202296
>
> Tom Schindl has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   8202296: Monocle MouseInput doesn't send keyboard modifiers in events.
>   
>   Fix whitespace errors

Looks good to me. I verified that the new test fails without your fix and 
passes with your fix.

In case anyone else is looking at this and wants to run the test, it isn't 
enabled by default, so you need to run it
like this:

gradle -PUNSTABLE_TEST=true -PFULL_TEST=true -PUSE_ROBOT=true \
:systemTests:test --tests test.robot.com.sun.glass.ui.monocle.RobotTest

-

Marked as reviewed by kcr (Lead).

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

Re: [Rev 02] RFR: 8191758: Match WebKit's font weight rendering with JavaFX

[Kevin Rushforth]

On Tue, 21 Apr 2020 16:34:11 GMT, Bhawesh Choudhary 
 wrote:

>> As per JavaFx 700 font weight is considered to be bold but webkit is using 
>> 600 font weight for text to become bold. to
>> fix issue, use boldWeightValue() function which uses 700 font weight rather 
>> than isFontWeightBold() which compare
>> against 600 font weight.
>
> Bhawesh Choudhary has refreshed the contents of this pull request, and 
> previous commits have been removed. The
> incremental views will show differences compared to the previous content of 
> the PR.

The fix and test look good. I confirm that your new test fails without your fix 
and passes with your fix.

I left one style comment and will approve once you fix that.

modules/javafx.web/src/test/java/test/javafx/scene/web/WebViewTest.java line 
111:

> 110: );
> 111: submit(()->{
> 112: assertFalse("Font weight test failed ",

Minor: there should be a space before and after the `->`

-

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

Re: RFR: 8171898: [WebView] ScrollBarWidget thickness calculation is not correct

[Kevin Rushforth]

On Mon, 27 Apr 2020 08:17:50 GMT, Arun Joseph  wrote:

> After [JDK-8157900](https://bugs.openjdk.java.net/browse/JDK-8157900), 
> initializeThickness() moved from
> ScrollBarThemeImpl to ScrollBarWidget but the implementation was kept the 
> same by using testSBRef to call
> setThickness() for setting ScrollBarTheme.thickness.  Modifying 
> initializeThickness() to set ScrollBarTheme.thickness
> once and removing testSBRef.

While this might be the right fix, I don't think the evaluation is right. In 
particular, this doesn't look related to
[JDK-8157900](https://bugs.openjdk.java.net/browse/JDK-8157900). That 
refactoring fix was behavior neutral in terms of
the object graph -- it uses an accessor pattern rather than the removed public 
`impl_` methods for encapsulation, but
intentionally didn't change anything related to which instance of which objects 
are used. That fix was not backported
to 8u and yet this bug is listed as affecting 8u.

So, what I'd like to see in the evaluation is a description of the root cause, 
and a brief explanation of the fix.

Can you also provide a test case that fails without the fix and passes with the 
fix? I don't see one in JBS that you
can leverage, so you may need to create one.

-

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

Re: RFR: 8241999: ChoiceBox: incorrect toggle selected for uncontained

[Kevin Rushforth]

On Tue, 28 Apr 2020 15:36:53 GMT, Jeanette Winzenburg  
wrote:

> The issue is that the toggles is not reliably unselected if an uncontained 
> value is set.
> 
> The root is ChoiceBoxSelectionModel which doesn't update the index on 
> selecting an uncontained item, in particular it
> fails to keep the invariant:
>  assertEquals(getItems().indexOf(selectedItem), selectedIndex);
> 
> The fix here is to override select(item) to guarantee the assert.
> 
> Added/removed ignore from tests that failed before and pass after the fix. 
> All other tests are passing before and after.

@aghaisas can you review this? It looks pretty simple so one reviewer should be 
enough, unless you spot something where
you want a second pair of eyes.

-

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

Re: RFR: 8244042: Change JavaFX release version in 11-dev to 11.0.8

[Kevin Rushforth]

+1

On 4/28/2020 1:19 PM, Johan Vos wrote:

Hi Kevin,

Please review http://cr.openjdk.java.net/~jvos/8244042/webrev.00/
which fixes https://bugs.openjdk.java.net/browse/JDK-8244042

Thanks,

- Johan

Re: [Rev 01] RFR: 8198402: ToggleButton.setToggleGroup causes memory leak when button is removed via ToggleGroup.getToggles()

[Kevin Rushforth]

On Sun, 19 Apr 2020 09:51:07 GMT, Jesper Skov 
 wrote:

>> Make the two ways of associating a ToggleButton with a ToggleGroup interact 
>> correctly.
>> 
>> This fixes https://bugs.openjdk.java.net/browse/JDK-8198402
>
> Jesper Skov has updated the pull request incrementally with three additional 
> commits since the last revision:
> 
>  - Fail instead of print message
>  - addedToggles list is final
>  - Leave unrelated file alone

Marked as reviewed by kcr (Lead).

-

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

RFR: 8244042: Change JavaFX release version in 11-dev to 11.0.8

[Johan Vos]

Hi Kevin,

Please review http://cr.openjdk.java.net/~jvos/8244042/webrev.00/
which fixes https://bugs.openjdk.java.net/browse/JDK-8244042

Thanks,

- Johan

Re: [Integrated] RFR: 8242077: Add information about HTTP/2 and HttpClient usage in WebEngine

[Abhinay Agarwal]

On Sun, 19 Apr 2020 08:12:07 GMT, Abhinay Agarwal 
 wrote:

> Update WebEngine's Javadoc to add information on how it switches to 
> HttpClient instead of URLConnection in JavaFX 14
> when used with JDK 12 or later.
> Identification of the correct client is important as both these clients may 
> offer different ways to achieve a task. One
> such task can be bypassing insecure HTTPS connection.

This pull request has now been integrated.

Changeset: e30049f0
Author:Abhinay Agarwal 
Committer: Kevin Rushforth 
URL:   https://git.openjdk.java.net/jfx/commit/e30049f0
Stats: 6 lines in 1 file changed: 0 ins; 6 del; 0 mod

8242077: Add information about HTTP/2 and HttpClient usage in WebEngine

Reviewed-by: kcr, ghb

-

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

RFR: 8241999: ChoiceBox: incorrect toggle selected for uncontained

[Jeanette Winzenburg]

The issue is that the toggles is not reliably unselected if an uncontained 
value is set.

The root is ChoiceBoxSelectionModel which doesn't update the index on selecting 
an uncontained item, in particular it
fails to keep the invariant:

 assertEquals(getItems().indexOf(selectedItem), selectedIndex);

The fix here is to override select(item) to guarantee the assert.

Added/removed ignore from tests that failed before and pass after the fix. All 
other tests are passing before and after.

-

Commit messages:
 - 8241999: ChoiceBox: incorrect toggle selected for uncontained

Changes: https://git.openjdk.java.net/jfx/pull/200/files
 Webrev: https://webrevs.openjdk.java.net/jfx/200/webrev.00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8241999
  Stats: 55 lines in 2 files changed: 44 ins; 7 del; 4 mod
  Patch: https://git.openjdk.java.net/jfx/pull/200.diff
  Fetch: git fetch https://git.openjdk.java.net/jfx pull/200/head:pull/200

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

Re: RFR: 8242505: Some WebKit tests might fail because Microsoft libraries are not loaded

[Guru Hb]

On Wed, 22 Apr 2020 18:07:09 GMT, Kevin Rushforth  wrote:

> This PR will allow javafx.web unit tests to run to completion and pass on 
> Windows with Visual Studio 2019.
> 
> Two of the WebKit tests load the native WebKit library without initializing 
> the JavaFX runtime. This will lead to test
> failures and will hang the test run if those tests are run before the other 
> tests, and if the system doesn't have all
> the needed runtime libraries.   The simple fix is to call the Toolkit method 
> to load those libraries. This is only
> needed for unit tests that don't already initialize the JavaFX runtime (e.g., 
> by calling `Platform::startup` or
> `Application::launch`) and call internal methods that execute native code as 
> part of the test.

Looks good to me

-

Marked as reviewed by ghb (Reviewer).

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

Re: [Integrated] RFR: 8242505: Some WebKit tests might fail because Microsoft libraries are not loaded

[Kevin Rushforth]

On Wed, 22 Apr 2020 18:07:09 GMT, Kevin Rushforth  wrote:

> This PR will allow javafx.web unit tests to run to completion and pass on 
> Windows with Visual Studio 2019.
> 
> Two of the WebKit tests load the native WebKit library without initializing 
> the JavaFX runtime. This will lead to test
> failures and will hang the test run if those tests are run before the other 
> tests, and if the system doesn't have all
> the needed runtime libraries.   The simple fix is to call the Toolkit method 
> to load those libraries. This is only
> needed for unit tests that don't already initialize the JavaFX runtime (e.g., 
> by calling `Platform::startup` or
> `Application::launch`) and call internal methods that execute native code as 
> part of the test.

This pull request has now been integrated.

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

8242505: Some WebKit tests might fail because Microsoft libraries are not loaded

Reviewed-by: ghb

-

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

Re: [Rev 04] RFR: 8241840: Memoryleak: Closed focused Stages are not collected with Monocle.

[Florian Kirmaier]

On Mon, 27 Apr 2020 15:09:17 GMT, Ambarish Rapte  wrote:

>> Florian Kirmaier has updated the pull request incrementally with one 
>> additional commit since the last revision:
>> 
>>   JDK-8241840
>>   The tests are now reused for native and monocle tests.
>
> modules/javafx.graphics/src/main/java/com/sun/glass/ui/Window.java line 1325:
> 
>> 1324: this.isFocused = focused;
>> 1325: if (this.isFocused && this.isVisible) {
>> 1326: setFocusedWindow(this);
> 
> On my Window10 machine, with this change, `Window.focusedWindow` remains 
> `null` even after the first window (I have not
> verified with multiple windows though) is shown onto the screen and is 
> focused. And It continues to remain `null` until
> some mouse or key action is performed on the window. I am not sure if this 
> causes any side effects. It looks like the
> `Window.focusedWindow` is mostly(only) used for Monocle. Can you please 
> confirm the behavior that
> `Window.focusedWindow` remain `null` and check for any side effects.

As mentioned - I don't have a good setup to test this code on Windows.

But I've checked where focusedWindow/getFocusedWindow is used, and I can verify 
your assumption. I've searched through
the whole project and the variable is only used in the MonocleCode.

The fact that focusedWindow get's sometimes set is probably the cause of the 
irregular happening memoryleak on Window.

-

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

Re: Automate copyright year?

[Jeanette Winzenburg]

sounds good :)

Zitat von Kevin Rushforth :


Then we might decide not to request a change in review if only the

copyright is not updated?

Yes, this is a good idea. I don't generally flag this in my reviews,  
for example. The exceptions would be: 1) if a new file springs to  
life with the wrong initial copyright year; 2) if an incorrect  
update was made to a copyright year.


-- Kevin


On 4/28/2020 4:18 AM, Jeanette Winzenburg wrote:


Okay, thanks for the info :)

Then we might decide not to request a change in review if only the  
copyright is not updated? If the second reviewers does it after the  
first already approved, the first has to re-approve - additional  
bureaucratic work and might lead to a bit of time lag across  
continents. Not such a big deal, but if it doesn't really matter ..


-- Jeanette

Zitat von Kevin Rushforth :

Not that I know of. FWIW, I'm not too bothered by this, since I  
run a script 2-3 times a year to update the copyright years for  
those files fixed in the current year.


-- Kevin


On 4/13/2020 3:48 AM, Jeanette Winzenburg wrote:


Seeing that missing to update of copyright year in the header is  
rather common (in my own pull requests as well as in others :) is  
there any means to do so automatically or some source tool (for  
me that would be for Eclipse) that would do on request?


-- Jeanette

Next steps ? (Re: An attempt of a CSR draft ... (Re: A new WIP (PR # 192) (Re: WIP version with PI compile (Re: A WIP for JDK-8238080 for review/discussion (Re: "Internal review ID 9063426: "FXMLLoade

[Rony G. Flatscher]

Hi Kevin,

what should be the next steps?

Should I remove "WIP" from the title in 
 and add the CSR
draft text of my last e-mail as a "CSR" comment with PR # 192, thereby 
requesting it to be added to
?

Please advise.

TIA,

---rony

P.S.: This is the RFE:

  * RFE (2020-01-24): 


These are the three versions (all with appropriate unit tests) that I came up 
with chronologically
to implement the RFE, would prefer the latest version (PR # 192):

  * Compile if Compilable implemented (2020-02-28): 

  * Compile if compile PI and Compilable is implemented (2020-04-11):

  * Compile with fallback, if Compilable is implemented, compile PI for 
fine-grained control
(2020-04-14): 


On 22.04.2020 20:01, Rony G. Flatscher wrote:
> Hi Kevin,
>
> as I am not able to file a CSR with the issue you suggested to come up with a 
> draft, so here it goes:
>
> Summary
> ===
> Have javafx.fxml.FXMLLoader compile FXML scripts before evaluating them, 
> if the script engine
> implements the javax.script.Compilable interface to speed up execution. 
> In case compilation
> throws a javax.script.ScriptException fall back to evaluating the 
> uncompiled script. Allow
> control of script compilation with a "compile" PI for FXML files.
>
> Problem
> ===
> javafx.fxml.FXMLLoader is able to execute scripts in Java script languages
> (javax.script.ScriptEngine implementations) referred to or embedded in a 
> FXML file.
>
> If a script engine implements the javax.script.Compilable interface, then 
> such scripts could be
> compiled and the resulting javax.script.CompiledScript could be executed 
> instead using its
> eval() methods.
>
> Evaluating the javax.script.CompiledScript objects may help speed up the 
> execution of script
> invocations, especially for scripts defined for event attributes in FXML 
> elements (e.g. like
> onMouseMove) which may be repetitively invoked and evaluated.
>
> Solution 
> 
> Before evaluating the script code test whether the 
> javax.script.ScriptEngine implements
> javax.script.Compilable. If so, compile the script to a 
> javax.script.CompiledScript object first
> and then use its eval() method to evaluate the script, otherwise continue 
> to use the
> javax.script.ScriptEngine's eval() method instead. Should compilation of 
> a script yield
> (unexpectedly) a javax.script.ScriptException then fall back to using the
> javax.script.ScriptEngine's eval() method. A new process instruction 
> "compile" allows control of
> the compilation of scripts ("true" sets compilation on, "false" to set 
> compilation off) in FXML
> files.
>
> Specification
> =
> If a javax.script.ScriptEngine implements the javax.script.Compilable 
> interface, then use its
> compile() method to compile the script to a javax.script.CompiledScript 
> object and use its
> eval() method to run the script. In the case that the compilation throws 
> (unexpectedly) a
> javax.script.ScriptException log a warning and fall back to using the
> javax.script.ScriptEngine's eval() method instead.
> To allow setting this feature off and on while processing the FXML file a 
> "compile" process
> instruction ("" or "") gets defined that 
> allows to turn
> compilation off and on throughout a FXML file.
>
> Having never seen a real CSR I hope that this matches what is expected and is 
> helpful for
> assessment. If not please advise (got the name of these fields from [1]).
>
> ---
>
> Also added brief information about the respective test units (what they test 
> and yield) in the WIP  [2].
>
> ---rony
>
> [1] "CSR-FAQ": 
>
> [2] "WIP: Script compilable+compile PI+fallback: 8238080: FXMLLoader: if 
> script engines implement
> javax.script.Compilable compile scripts #192":  
> 
>
>
> On 20.04.2020 14:58, Rony G. Flatscher wrote:
>> There is a new WIP at :
>>
>> This WIP adds the ability for a fallback in case compilation of scripts 
>> fails, in which case a
>> warning gets issued about this fact and evaluation of the script will be 
>> done without
>> compilation. Because of the fallback scripts get compiled with this 
>> version by default. It
>> extends PR 187 #187.
>>
>> To further ease spotting scripts that cause a ScriptException a message 
>> in the form of
>> "filename: caused ScriptException" gets added to the exception handling 
>> in either of the three
>> locations: an error message, a stack trace or a 

Re: Automate copyright year?

[Kevin Rushforth]

> Then we might decide not to request a change in review if only the 
copyright is not updated?


Yes, this is a good idea. I don't generally flag this in my reviews, for 
example. The exceptions would be: 1) if a new file springs to life with 
the wrong initial copyright year; 2) if an incorrect update was made to 
a copyright year.


-- Kevin


On 4/28/2020 4:18 AM, Jeanette Winzenburg wrote:


Okay, thanks for the info :)

Then we might decide not to request a change in review if only the 
copyright is not updated? If the second reviewers does it after the 
first already approved, the first has to re-approve - additional 
bureaucratic work and might lead to a bit of time lag across 
continents. Not such a big deal, but if it doesn't really matter ..


-- Jeanette

Zitat von Kevin Rushforth :

Not that I know of. FWIW, I'm not too bothered by this, since I run a 
script 2-3 times a year to update the copyright years for those files 
fixed in the current year.


-- Kevin


On 4/13/2020 3:48 AM, Jeanette Winzenburg wrote:


Seeing that missing to update of copyright year in the header is 
rather common (in my own pull requests as well as in others :) is 
there any means to do so automatically or some source tool (for me 
that would be for Eclipse) that would do on request?


-- Jeanette

Re: [Rev 01] RFR: 8087555: [ChoiceBox] uncontained value not shown

[Ambarish Rapte]

On Tue, 28 Apr 2020 09:38:56 GMT, Jeanette Winzenburg  
wrote:

>> The issue is that ChoiceBoxSkin
>> a) doesn't update the text of the label if the value is not contained in the 
>> items
>> b) doesn't respect converter for label text
>> 
>> Fixed by
>> - listening to value changes to update the label
>> - removing ad-hoc updates (not needed), added update on converter change
>> - passing all label updates through converter
>> 
>> Added test for text updates that failed before the fix and pass after (note: 
>> there were no tests for the display text,
>> so for coveragy, contains also tests that passed before as well as after)
>
> Jeanette Winzenburg has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   fixed copyright year

Marked as reviewed by arapte (Reviewer).

-

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

Re: RFR: 8242077: Add information about HTTP/2 and HttpClient usage in WebEngine

[Guru Hb]

On Sun, 19 Apr 2020 08:12:07 GMT, Abhinay Agarwal 
 wrote:

> Update WebEngine's Javadoc to add information on how it switches to 
> HttpClient instead of URLConnection in JavaFX 14
> when used with JDK 12 or later.
> Identification of the correct client is important as both these clients may 
> offer different ways to achieve a task. One
> such task can be bypassing insecure HTTPS connection.

looks good to me

-

Marked as reviewed by ghb (Reviewer).

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

Re: Automate copyright year?

[Jeanette Winzenburg]

Okay, thanks for the info :)

Then we might decide not to request a change in review if only the  
copyright is not updated? If the second reviewers does it after the  
first already approved, the first has to re-approve - additional  
bureaucratic work and might lead to a bit of time lag across  
continents. Not such a big deal, but if it doesn't really matter ..


-- Jeanette

Zitat von Kevin Rushforth :

Not that I know of. FWIW, I'm not too bothered by this, since I run  
a script 2-3 times a year to update the copyright years for those  
files fixed in the current year.


-- Kevin


On 4/13/2020 3:48 AM, Jeanette Winzenburg wrote:


Seeing that missing to update of copyright year in the header is  
rather common (in my own pull requests as well as in others :) is  
there any means to do so automatically or some source tool (for me  
that would be for Eclipse) that would do on request?


-- Jeanette

Re: [Rev 01] RFR: 8087555: [ChoiceBox] uncontained value not shown

[Ajit Ghaisas]

On Tue, 28 Apr 2020 09:38:56 GMT, Jeanette Winzenburg  
wrote:

>> The issue is that ChoiceBoxSkin
>> a) doesn't update the text of the label if the value is not contained in the 
>> items
>> b) doesn't respect converter for label text
>> 
>> Fixed by
>> - listening to value changes to update the label
>> - removing ad-hoc updates (not needed), added update on converter change
>> - passing all label updates through converter
>> 
>> Added test for text updates that failed before the fix and pass after (note: 
>> there were no tests for the display text,
>> so for coveragy, contains also tests that passed before as well as after)
>
> Jeanette Winzenburg has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   fixed copyright year

Marked as reviewed by aghaisas (Reviewer).

-

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

Re: [Rev 01] RFR: 8087555: [ChoiceBox] uncontained value not shown

[Jeanette Winzenburg]

> The issue is that ChoiceBoxSkin
> a) doesn't update the text of the label if the value is not contained in the 
> items
> b) doesn't respect converter for label text
> 
> Fixed by
> - listening to value changes to update the label
> - removing ad-hoc updates (not needed), added update on converter change
> - passing all label updates through converter
> 
> Added test for text updates that failed before the fix and pass after (note: 
> there were no tests for the display text,
> so for coveragy, contains also tests that passed before as well as after)

Jeanette Winzenburg has updated the pull request incrementally with one 
additional commit since the last revision:

  fixed copyright year

-

Changes:
  - all: https://git.openjdk.java.net/jfx/pull/191/files
  - new: https://git.openjdk.java.net/jfx/pull/191/files/e7f9e398..9b9a28a9

Webrevs:
 - full: https://webrevs.openjdk.java.net/jfx/191/webrev.01
 - incr: https://webrevs.openjdk.java.net/jfx/191/webrev.00-01

  Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod
  Patch: https://git.openjdk.java.net/jfx/pull/191.diff
  Fetch: git fetch https://git.openjdk.java.net/jfx pull/191/head:pull/191

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

Re: RFR: 8087555: [ChoiceBox] uncontained value not shown

[Ajit Ghaisas]

On Mon, 27 Apr 2020 11:19:33 GMT, Ambarish Rapte  wrote:

>> The issue is that ChoiceBoxSkin
>> a) doesn't update the text of the label if the value is not contained in the 
>> items
>> b) doesn't respect converter for label text
>> 
>> Fixed by
>> - listening to value changes to update the label
>> - removing ad-hoc updates (not needed), added update on converter change
>> - passing all label updates through converter
>> 
>> Added test for text updates that failed before the fix and pass after (note: 
>> there were no tests for the display text,
>> so for coveragy, contains also tests that passed before as well as after)
>
> Marked as reviewed by arapte (Reviewer).

The fix is OK.

Test file header should have year 2020 and not 2019.

-

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

Re: [Rev 03] RFR: 8242548: Honor line spacing in Labeled reflow calculation

[Ajit Ghaisas]

On Mon, 27 Apr 2020 14:02:42 GMT, John Hendrikx  wrote:

>> This is a solution for 8242548.  There was zero test coverage, so I added a 
>> few tests for this as well.
>
> John Hendrikx has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Fix typo in comment

Marked as reviewed by aghaisas (Reviewer).

-

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