JDK-8200418 : webPage.executeCommand("removeFormat", null) removes the style of the body element
  • Type: Bug
  • Component: javafx
  • Sub-Component: web
  • Affected Version: 8u172,10,openjfx11
  • Priority: P3
  • Status: Resolved
  • Resolution: Fixed
  • OS: generic
  • CPU: x86_64
  • Submitted: 2018-03-28
  • Updated: 2020-01-31
  • Resolved: 2018-05-03
The Version table provides details related to the release that this issue/RFE will be addressed.

Unresolved : Release in which this issue/RFE will be addressed.
Resolved: Release in which this issue/RFE has been resolved.
Fixed : Release in which this issue/RFE has been fixed. The release containing this fix may be available for download as an Early Access Release or a General Availability Release.

To download the current JDK release, click here.
JDK 10 JDK 8 Other
10.0.2Fixed 8u181Fixed openjfx11Fixed
Related Reports
Duplicate :  
Duplicate :  
Relates :  
Relates :  
Relates :  
Relates :  
Description
FULL PRODUCT VERSION :
java version "1.8.0_172-ea"
Java(TM) SE Runtime Environment (build 1.8.0_172-ea-b03)
Java HotSpot(TM) 64-Bit Server VM (build 25.172-b03, mixed mode)


ADDITIONAL OS VERSION INFORMATION :
macOS 10.12.6

A DESCRIPTION OF THE PROBLEM :
When using webPage.executeCommand("removeFormat", null) the formatting of the selected text should be cleared. Instead the style attribute of the body element is removed.

REGRESSION.  Last worked in version 8u162

ADDITIONAL REGRESSION INFORMATION: 
java version "1.8.0_162"
Java(TM) SE Runtime Environment (build 1.8.0_162-b12)
Java HotSpot(TM) 64-Bit Server VM (build 25.162-b12, mixed mode)


STEPS TO FOLLOW TO REPRODUCE THE PROBLEM :
Create a view with a HTMLEditor and a button to execute the "removeFormat" command and another button "print to console" for debugging. Then view a text that contains a style in the body:

public class Controller implements Initializable {
    public HTMLEditor htmlEditor;

    @Override
    public void initialize(URL location, ResourceBundle resources) {
        htmlEditor.setHtmlText("<html>\n" +
                "<head>\n" +
                "    <meta http-equiv=\"Content-Type\" content=\"text/html; charset=utf-8\">\n" +
                "</head>\n" +
                "<body style=\"font-family: san-serif\">\n" +
                "<p>Test</p>\n" +
                "</body>\n" +
                "</html>");
    }

    public void onRemoveFormat(ActionEvent actionEvent) {
        WebView webView = (WebView) htmlEditor.lookup("WebView");
        WebPage webPage = Accessor.getPageFor(webView.getEngine());
        webPage.executeCommand("removeFormat", null);
    }

    public void onPrintConsole(ActionEvent actionEvent) {
        System.out.println(htmlEditor.getHtmlText());
    }
}

Now start the program and click the "Print to console" to see this:

<html dir="ltr"><head>
    <meta http-equiv="Content-Type" content="text/html; charset=utf-8">
</head>
<body style="font-family: san-serif" contenteditable="true">
<p>Test</p>

</body></html>


Then select the "Test" text and click on "remove format". Afterward click "Print to console" again. Java version 1.8.0_172 prints this:

<html dir="ltr"><head>
    <meta http-equiv="Content-Type" content="text/html; charset=utf-8">
</head>
<body contenteditable="true">
<p>Test</p>

</body></html>

As you can see the style attribute has been removed from the body tag. However in version 1.8.0_162 it worked correct, the body tag was not modified, the style tag remained there.

EXPECTED VERSUS ACTUAL BEHAVIOR :
EXPECTED -
"removeFormat" should only modify the selected text and in this example the text has no formatting so "removeFormat" should do nothing.
ACTUAL -
The "style" attribute of the body tag is removed, which is incorrect.

REPRODUCIBILITY :
This bug can be reproduced always.

---------- BEGIN SOURCE ----------
public class Controller implements Initializable {
    public HTMLEditor htmlEditor;

    @Override
    public void initialize(URL location, ResourceBundle resources) {
        htmlEditor.setHtmlText("<html>\n" +
                "<head>\n" +
                "    <meta http-equiv=\"Content-Type\" content=\"text/html; charset=utf-8\">\n" +
                "</head>\n" +
                "<body style=\"font-family: san-serif\">\n" +
                "<p>Test</p>\n" +
                "</body>\n" +
                "</html>");
    }

    public void onRemoveFormat(ActionEvent actionEvent) {
        WebView webView = (WebView) htmlEditor.lookup("WebView");
        WebPage webPage = Accessor.getPageFor(webView.getEngine());
        webPage.executeCommand("removeFormat", null);
    }

    public void onPrintConsole(ActionEvent actionEvent) {
        System.out.println(htmlEditor.getHtmlText());
    }
}


and


<?xml version="1.0" encoding="UTF-8"?>

<?import javafx.scene.control.*?>
<?import javafx.scene.web.*?>
<?import java.lang.*?>
<?import javafx.scene.layout.*?>
<?import javafx.geometry.Insets?>
<?import javafx.scene.layout.GridPane?>
<?import javafx.scene.control.Button?>
<?import javafx.scene.control.Label?>

<BorderPane maxHeight="-Infinity" maxWidth="-Infinity" minHeight="-Infinity" minWidth="-Infinity" prefHeight="433.0" prefWidth="671.0" xmlns="http://javafx.com/javafx/8" xmlns:fx="http://javafx.com/fxml/1" fx:controller="sample.bug2_htmleditor.Controller">
   <center>
      <HTMLEditor fx:id="htmlEditor" />
   </center>
   <bottom>
      <ToolBar prefHeight="40.0" prefWidth="200.0" BorderPane.alignment="CENTER">
        <items>
          <Button mnemonicParsing="false" onAction="#onRemoveFormat" text="Clear Formatting" />
          <Button mnemonicParsing="false" onAction="#onPrintConsole" text="Print to console" />
        </items>
      </ToolBar>
   </bottom>
</BorderPane>


---------- END SOURCE ----------


Comments
Originally I was thinking we didn't need a 10u-backport, since we haven't been backporting most fixes to 10u, but in this case, it was a serious regression introduced in 10, so we should backport it to 10u.
03-05-2018

[~mbilla] we need 10u back port in case this is reproducible on 10.0.1
03-05-2018

Changeset: da551ab4cec1 Author: mbilla Date: 2018-05-03 14:56 +0530 URL: http://hg.openjdk.java.net/openjfx/jfx-dev/rt/rev/da551ab4cec1
03-05-2018

+1
03-05-2018

+1
02-05-2018

webrev: http://cr.openjdk.java.net/~mbilla/8200418/webrev.02/
02-05-2018

+1 with minor nits: -You need not have a separate variable for selectall command. Can have the contents inline in executescript. -.As there is no visual implication, we don't need to append newlines at the end for the content set through htmlEditor.setHtmlText
02-05-2018

1. Please remove the following now-unused import, so that the change to HTMLEditorSkin is a true backout: 72 import com.sun.webkit.dom.HTMLDocumentImpl; 2. In the test file, you can leave the existing comment before the @Ignore, but you also need to mention the newly cloned bug ID that needs to be fixed before the test can be re-enabled. Also, please put that JDK bug ID as a string parameter to the @Ignore, like this: @Ignore("JDK-8202542") Otherwise, looks good. I confirm that the new test fails without your fix and passes with your fix.
02-05-2018

webrev: http://cr.openjdk.java.net/~mbilla/8200418/webrev.01/ 1. The fix for JDK-8090011 is backed out (which is causing regression). But i retained the test case "checkFocusChange" (currently under @Ignore). The issue JDK-8090011 is marked as "Fix failed" and new issue (JDK-8202542) is created to address JDK-8090011. 2. Since we backed out the fix for JDK-8090011, the test case written for JDK-8088769 (checkStyleWithCSS) is failing, I corrected this issue in webrev. 3. Test case (checkStyleProperty) for JDK-8200418 is added in HTMLEditorTest.java.
02-05-2018

[~mbilla] But if setdesignmode("off") is indeed resulting in focusout, then you should be seeing the issue visually in JDK-8090011 even after the applying webrev.00. I feel it could be a timing issue of design mode being actually turned "on" inside WebCore and execution of tab.
26-04-2018

[~rkamath] Thanks for the suggestion. I checked with above change and looks like checkStyleWithCSS is passing now. But i believe that till now the test case checkStyleWithCSS is passing due to the combination of setDesignMode + fix for JDK-8088769 (which suppressed the bug in test case). Now that we removed setDesignMode, the bug in test case got exposed. Regarding other test case failure (checkFocusChange), With setDesignMode ON/OFF alternatively, looks like with "OFF" (means not editable), onfocusout is called which sets backgroundcolor to yellow. So one way to fix this failure is to modify the test case to check backgroud color with yellow instead of red. result.set("yellow".equals(webView.getEngine(). executeScript("document.body.style.backgroundColor"). toString())); [~rkamath] if above solution is fine, then i will send a webrev.
26-04-2018

I looked into HTMLEditorTest.checkStyleWithCSS failing, and it appears to be because of focus not being set inside the document for this test case. stylewithcss is independent of design mode and i believe this is a bug in test case. By making this change, the test passes irrespective of designmode for me. Util.runAndWait(() -> { + webView.getEngine().executeScript("document.body.focus();"); webView.getEngine().executeScript(editorCommand1); assertEquals(expectedHTML, htmlEditor.getHtmlText()); webView.getEngine().executeScript(editorCommand2); [~mbilla] could you please confirm with the above at your end as well. However, at the moment i still have not figured out how test case was working without setting the focus explicitly.
25-04-2018

Looks like there are 2 issues (JDK-8090011, JDK-8088769) which are related to the above suggested fix. 1. With new proposed fix by calling setDeisgnMode(off), the test case for JDK-8090011 is failing. 2. It seems the test case for JDK-8088769 is dependent on fix for JDK-8090011. When i moved setDesignMode before executing tab command, the test case for JDK-8088769 fails. Basically, what setDeisgnMode does is just force to recalculate the style for document. void Document::setDesignMode(InheritedBool value) { m_designMode = value; for (Frame* frame = m_frame; frame && frame->document(); frame = frame->tree().traverseNext(m_frame)) frame->document()->scheduleForcedStyleRecalc(); }
25-04-2018

Code change looks fine. There is a system test which verifies JDK-8090011(HTMLEditorTest.java). You can ensure this passes along with the one you add.
17-04-2018

Can you provide a unit test for this?
17-04-2018

webrev: http://cr.openjdk.java.net/~mbilla/8200418/webrev.00/ Verified the duplicated issue JDK-8200416 with above webrev.
17-04-2018

Thanks [~dkumar] for the suggestion.. I tested with below change and looks like JDK-8200418 is not reproduced (but one case still fails when we press tab and remove format). I tried by disabling Design mode to "off" after execution of tab command and no side effect was observed. diff -r 15fcabec7e20 modules/javafx.web/src/main/java/javafx/scene/web/HTMLEditorSkin.java --- a/modules/javafx.web/src/main/java/javafx/scene/web/HTMLEditorSkin.java Fri Mar 02 21:36:35 2018 +0530 +++ b/modules/javafx.web/src/main/java/javafx/scene/web/HTMLEditorSkin.java Tue Apr 17 17:20:21 2018 +0530 @@ -306,6 +306,7 @@ executeCommand(INDENT.getCommand(), null); } else { + setDesignMode("on"); executeCommand(INSERT_TAB.getCommand(), null); + setDesignMode("off"); } } @@ -448,7 +449,6 @@ if (newValue.doubleValue() == totalWork) { cachedHTMLText = null; Platform.runLater(() -> { - setDesignMode("on"); setContentEditable(true); updateToolbarState(true); updateNodeOrientation();
17-04-2018

Thanks [~dkumar] for the confirmation.. [~pmangal] Can you please retest JDK-8200416 and confirm whether JDK-8200416 is repro. on 9, 10 and 11. Below pasted are your results from JDK-8200416. I expect the issue JDK-8200416 should fail at least in 10 and 11. -------------------------------- 8u161-b12 : Pass 8u162-b12 : Pass 8u171-b11 : Pass 8u172-b01 : Fail <-- regression 8u172-b03 : Fail 8u172-b11 : Fail 9.0.4 : Pass 10+46 : Pass 11-ea+06 : Pass -----------------------------
16-04-2018

[~mbilla], [~rkamath], Issue mentioned in the https://bugs.openjdk.java.net/browse/JDK-8200416 is not reproducible after reverting the patch for https://bugs.openjdk.java.net/browse/JDK-8090011 .
16-04-2018

I just reverted the patch for JDK-8090011 locally in latest JFX-Dev and the problem is not seen. So, looks like a side effect of this.
16-04-2018

Thanks [~rkamath] .. The other issue JDK-8200416 also looks similar to JDK-8200418. But as per [~pmangal] comments in JDK-8200416, issue is not repro on 9, 10 and 11. Need to check JDK-8200416 again ..
16-04-2018

Removed and attached modified 'Controller.java' with public api's. Issue is reproducible in JDK 8u172, 10+46, 11-ea+6.
05-04-2018

Requested Submitter for test program that doesn't use proprietary classes. Marking as incomplete pending more information.
03-04-2018

We will need a test program using public API that shows the problem. WebPage is not public API so this usage is unsupported.
29-03-2018

This is similar to JDK-8200416, which is regression in 8u172 in both Windows and macOS. But reproducible test case using private package "com.sun.javafx.webkit./com.sun.webkit.WebPage" , which is prohibited from JDK 9 onward. Windows 10, 64-bit JDK results -------------------------------- 8u161-b12 : Pass 8u162-b12 : Pass 8u171-b11 : Pass 8u172-b01 : Fail <-- regression 8u172-b03 : Fail 8u172-b11 : Fail -------------------------------- Steps to reproduce: 1. Run test case and click the "Print to console" 2. Then select the "Test" text and click on "Clear Formatting". Now again click "Print to console". 3. Compare console logs where style has been removed from body after calling 'removeFormat'.
29-03-2018

Does this affect JDK 10 or 11-ea?
29-03-2018