RFR: 8090547: Allow for transparent backgrounds in WebView [v4]

[Kevin Rushforth]

On Tue, 24 Aug 2021 22:12:01 GMT, Jose Pereda <jpereda at 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:
> 
>   Update cssref.html

I left a couple more comments on the API docs, and a few comments / questions on the implementation. I've tested this on Windows, but still need to test on the other two platforms.

modules/javafx.graphics/src/main/java/com/sun/prism/es2/ES2Graphics.java line 70:

> 68:         context.updateCompositeMode(CompositeMode.CLEAR);
> 69:         Paint oldPaint = getPaint();
> 70:         setPaint(Color.TRANSPARENT); // any color will do...

Is this change necessary? If so, then the comment is probably wrong.

modules/javafx.web/src/main/java/com/sun/javafx/webkit/prism/WCGraphicsPrismContext.java line 459:

> 457:     public void setClip(WCRectangle c) {
> 458:         if (!isOpaque) {
> 459:             clearRect((int)c.getX(), (int)c.getY(),

Are you sure that there are no ill effects from clearing the rectangle every time a clip is set on a non-opaque context? This seems like a surprising side effect.

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

> 613:             twkSetTransparent(frameID, isBackgroundTransparent());
> 614:             twkSetBackgroundColor(frameID, backgroundColor);
> 615:             repaintAll();

Is this needed unconditionally? Or only when the background is not opaque?

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

> 635:                 twkSetBackgroundColor(frameID, backgroundColor);
> 636:             }
> 637:             repaintAll();

Same question as above.

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

> 701:      * Specifies the background color of the web page.
> 702:      *
> 703:      * With this property, the WebView control's background

I recommend code style for `WebView`.

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

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

Typo: set --> sets

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

> 706:      *
> 707:      * However, if the HTML content being loaded set its own
> 708:      * background color, it will take precedence.

Maybe say the "that color" (instead of "it") will take precedence?

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

> 731:                     Color color = get();
> 732:                     page.setBackgroundColor(color != null ? color.hashCode() :
> 733:                             DEFAULT_PAGE_FILL.hashCode());

This is relying on an undocumented, implementation detail. The current implementation of `Color::hashCode` happens to do what you want, but it is not specified. It seems safer to create a utility method to do this.

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

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