On Wed, 8 Sep 2021 16:15:36 GMT, Jose Pereda <jper...@openjdk.org> wrote:

>> Currently, `WebPage` has already a public `setBackgroundColor()` method, but 
>> the class is not public. Therefore, public API is needed in `WebView` to 
>> allow developers access to it.
>> 
>> In line with the `fontSmoothingType` property, this PR provides public 
>> support for setting the background color of a WebPage, by adding a 
>> `pageFill` property, and a CSR is required.
>> 
>> The color for the background, that can be opaque, transparent or with any 
>> level of opacity, can be set via code or via CSS using `-fx-page-fill`.
>> 
>> Unit tests and a system test are provided.
>
> Jose Pereda has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Address feedback from reviewers

I left a few minor comments, but otherwise this all looks good to me.

Once you make the one requested change to remove the paragraph break, you can 
also update the CSR with that change and the two other changes I requested in 
the CSR, and then move the CSR to proposed.

modules/javafx.web/src/main/java/com/sun/webkit/WebPage.java line 97:

> 95: 
> 96:     private int fontSmoothingType;
> 97:     private int backgroundIntRgba = 0xFFFFFFFF;

Maybe add back the comment about this being Color.WHITE?

modules/javafx.web/src/main/java/com/sun/webkit/WebPage.java line 2589:

> 2587:     private static int getIntRgba(Color color) {
> 2588:         if (color == null) {
> 2589:             return -1;

Maybe: `return 0xFFFFFFFF;` ? or else assign `color = Color.WHITE;` and fall 
through?

modules/javafx.web/src/main/java/javafx/scene/web/WebView.java line 707:

> 705:      * level of transparency.
> 706:      *
> 707:      * <p>However, if the HTML content being loaded sets its own

I looked at the generated javadoc and this will read better without a paragraph 
break.

-------------

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

Reply via email to