On Tue, 5 Nov 2019 09:26:56 GMT, Arun Joseph <github.com+24428922+arun-jos...@openjdk.org> wrote:
> On Thu, 10 Oct 2019 14:56:19 GMT, Hadzic Samir <shad...@openjdk.org> wrote: > >> On Thu, 10 Oct 2019 12:19:18 GMT, Kevin Rushforth <k...@openjdk.org> wrote: >> >>> On Thu, 10 Oct 2019 09:12:57 GMT, Hadzic Samir <shad...@openjdk.org> wrote: >>> >>>> On Wed, 9 Oct 2019 20:09:58 GMT, Kevin Rushforth <k...@openjdk.org> wrote: >>>> >>>>> On Wed, 9 Oct 2019 16:09:58 GMT, Hadzic Samir <shad...@openjdk.org> wrote: >>>>> >>>>>> On Wed, 9 Oct 2019 16:09:07 GMT, Kevin Rushforth <k...@openjdk.org> >>>>>> wrote: >>>>>> >>>>>>> On Wed, 9 Oct 2019 16:09:06 GMT, Hadzic Samir <shad...@openjdk.org> >>>>>>> wrote: >>>>>>> >>>>>>>> Fix for https://github.com/javafxports/openjdk-jfx/issues/573 >>>>>>>> >>>>>>>> Issue on JDK bug tracking : >>>>>>>> https://bugs.openjdk.java.net/browse/JDK-8230231 >>>>>>>> >>>>>>>> I tried to add a test but I do not succeed at even running the >>>>>>>> existing Web tests.. I will need some help on that side.. >>>>>>>> >>>>>>>> ---------------- >>>>>>>> >>>>>>>> Commits: >>>>>>>> - e9df9db5: Adding double-quote for HTMLEditorSkin font-family >>>>>>>> >>>>>>>> Changes: https://git.openjdk.java.net/jfx/pull/12/files >>>>>>>> Webrev: https://webrevs.openjdk.java.net/jfx/12/webrev.00 >>>>>>>> Issue: https://bugs.openjdk.java.net/browse/JDK-8230231 >>>>>>>> Stats: 4 lines in 1 file changed: 0 ins; 0 del; 4 mod >>>>>>>> Patch: https://git.openjdk.java.net/jfx/pull/12.diff >>>>>>>> Fetch: git fetch https://git.openjdk.java.net/jfx >>>>>>>> pull/12/head:pull/12 >>>>>>> >>>>>>> @Maxoudela please edit the title as follows: >>>>>>> >>>>>>> 1. Remove the space before the `:` (that extra space is why jcheck >>>>>>> failed) >>>>>>> 2. Change the text of the title to match the JBS bug summary exactly. >>>>>>> You can edit the JBS bug summary if you feel it needs to be changed, >>>>>>> but in this case, the JBS bug has a title that is more in line with our >>>>>>> usual practice of having the bug title be descriptive of what the >>>>>>> problem is and not what the solution happens to be. >>>>>>> >>>>>>> As for unit tests, you will very likely need to add this as a system >>>>>>> test under `tests/system/src/main/test`. See >>>>>>> [tests/system/src/test/java/test/javafx/scene/web/HTMLEditorTest.java](https://github.com/openjdk/jfx/blob/master/tests/system/src/test/java/test/javafx/scene/web/HTMLEditorTest.java). >>>>>>> Presuming that you can add your test to that existing class, you would >>>>>>> run it as follows: >>>>>>> >>>>>>> gradle -PFULL_TEST=true :systemTests:test --tests HTMLEditorTest >>>>>> >>>>>> Thanks @kevinrushforth . I'm sorry for posting the Pull request like >>>>>> that, I will thoroughly read the contributing guidelines and updates my >>>>>> PR accordingly. >>>>>> >>>>>> I'll try to add a test asap, thanks for the pointer. >>>>> >>>>>> I'm sorry for posting the Pull request like that >>>>> >>>>> No problem. I mainly wanted to make sure that you knew why the RFR wasn't >>>>> sent. As for the note about the title matching, the contributing >>>>> guidelines don't mention that and I now realize that they should -- I'll >>>>> add that along with some other improvements I'll be making. >>>>> >>>>>> I'll try to add a test asap, thanks for the pointer. >>>>> >>>>> Great, thanks. >>>> >>>> Hum I do not succeed in running the existing test either. Here is my log. >>>> Apparently, the tests are failing because the WebView is null and not >>>> initialized. Do you have any clue of what I've been doing wrong? >>>> >>>> Should I try to update my gradle version and JDK version maybe? >>>> gradle -PFULL_TEST=true :systemTests:test --tests HTMLEditorTest >>>> Starting a Gradle Daemon (subsequent builds will be faster) >>>> >>>>> Configure project : >>>> MACOSX_MIN_VERSION = 10.9 >>>> MACOSX_SDK_PATH = >>>> /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.14.sdk >>>> ***************************************************************** >>>> Unsupported gradle version 4.8 in use. >>>> Only version 5.3 is supported. Use this version at your own risk >>>> ***************************************************************** >>>> gradle.gradleVersion: 4.8 >>>> OS_NAME: mac os x >>>> OS_ARCH: x86_64 >>>> JAVA_HOME: /Library/Java/JavaVirtualMachines/jdk-11.0.1.jdk/Contents/Home >>>> JDK_HOME: /Library/Java/JavaVirtualMachines/jdk-11.0.1.jdk/Contents/Home >>>> java.runtime.version: 11.0.1+13 >>>> java version: 11.0.1 >>>> java build number: 13 >>>> jdk.runtime.version: 11.0.1+13 >>>> jdk version: 11.0.1 >>>> jdk build number: 13 >>>> minimum jdk version: 11 >>>> minimum jdk build number: 28 >>>> XCODE version: Xcode10.1-MacOSX10.14+1.0 >>>> cmake version: 3.13.3 >>>> ninja version: 1.8.2 >>>> ant version: 1.10.5 >>>> HAS_JAVAFX_MODULES: false >>>> STUB_RUNTIME: >>>> /Library/Java/JavaVirtualMachines/jdk-11.0.1.jdk/Contents/Home >>>> CONF: Debug >>>> NUM_COMPILE_THREADS: 1 >>>> COMPILE_TARGETS: mac >>>> COMPILE_FLAGS_FILES: buildSrc/mac.gradle >>>> HUDSON_JOB_NAME: not_hudson >>>> HUDSON_BUILD_NUMBER: 0000 >>>> PROMOTED_BUILD_NUMBER: 0 >>>> PRODUCT_NAME: OpenJFX >>>> RELEASE_VERSION: 14 >>>> RELEASE_SUFFIX: -internal >>>> RELEASE_VERSION_SHORT: 14-internal >>>> RELEASE_VERSION_LONG: 14-internal+0-2019-10-10-110056 >>>> RELEASE_VERSION_PADDED: 14.0.0.0 >>>> MAVEN_VERSION: 14-internal+0-2019-10-10-110056 >>>> UPDATE_STUB_CACHE: false >>>> Building Webkit configuration /Release/ into >>>> /Users/shadzic/jfx/modules/javafx.web/build/mac >>>> module: project ':apps' (buildModule=NO) >>>> module: project ':base' (buildModule=YES) >>>> module: project ':controls' (buildModule=YES) >>>> module: project ':fxml' (buildModule=YES) >>>> module: project ':graphics' (buildModule=YES) >>>> module: project ':media' (buildModule=YES) >>>> module: project ':swing' (buildModule=YES) >>>> module: project ':swt' (buildModule=NO) >>>> module: project ':systemTests' (buildModule=NO) >>>> module: project ':web' (buildModule=YES) >>>> >>>>> Task :web:compileJava >>>> Note: Some input files use or override a deprecated API. >>>> Note: Recompile with -Xlint:deprecation for details. >>>> Note: Some input files use unchecked or unsafe operations. >>>> Note: Recompile with -Xlint:unchecked for details. >>>> >>>>> Task :web:compileShimsJava >>>> Note: Some input files use or override a deprecated API. >>>> Note: Recompile with -Xlint:deprecation for details. >>>> Note: Some input files use unchecked or unsafe operations. >>>> Note: Recompile with -Xlint:unchecked for details. >>>> >>>>> Task :web:compileTestJava >>>> Note: Some input files use unchecked or unsafe operations. >>>> Note: Recompile with -Xlint:unchecked for details. >>>> >>>>> Task :systemTests:compileTestJava >>>> Note: Some input files use or override a deprecated API. >>>> Note: Recompile with -Xlint:deprecation for details. >>>> Note: Some input files use unchecked or unsafe operations. >>>> Note: Recompile with -Xlint:unchecked for details. >>>> >>>>> Task :systemTests:test >>>> >>>> test.javafx.scene.web.HTMLEditorTest > checkStyleWithCSS FAILED >>>> java.lang.NullPointerException >>>> at >>>> test.javafx.scene.web.HTMLEditorTest.lambda$checkStyleWithCSS$7(HTMLEditorTest.java:192) >>>> >>>> test.javafx.scene.web.HTMLEditorTest > checkStyleProperty FAILED >>>> java.lang.NullPointerException >>>> at >>>> test.javafx.scene.web.HTMLEditorTest.lambda$checkStyleProperty$11(HTMLEditorTest.java:266) >>>> >>>> 3 tests completed, 2 failed, 1 skipped >>>> >>>>> Task :systemTests:test FAILED >>> >>> The test failure is almost certainly due to not having the native WebKit >>> libraries. Two options: >>> >>> 1. You can build WebKit yourself (it's fairly painless on Mac) by running >>> gradle with `-PCOMPILE_WEBKIT=true` >>> 2. You can download libjfxwebkit.dylib from the openjfx14+1 EA build and >>> put it in `../caches/modular-sdk/modules_libs/javafx.web/libjfxwebkit.dylib` >> >> Allright, I've been able to build the WebKit. >> >> I am now trying to add my test. I took some snippets from MiscellaneousTest >> and add my method to HTMLEditorTest: >> >> /** >> * @test >> * @bug 8230231 >> * FIXME >> */ >> @Test >> public void checkFontFamily() throws Exception { >> final FontFaceTestHelper fontFaceHelper = new >> FontFaceTestHelper("src/test/resources/test/javafx/scene/web/Ahem.ttf"); >> >> >> final CountDownLatch editorStateLatch = new CountDownLatch(1); >> final AtomicReference<String> result = new AtomicReference<>(); >> Util.runAndWait(() -> { >> webView.getEngine().loadContent("<body>\n" + >> "<span id='probe1' style='font-size: 100px;'>Toto</span> >> Tutu\n" + >> "</body>\n", "text/html"); >> >> final JSObject window = (JSObject) >> webView.getEngine().executeScript("window"); >> assertNotNull(window); >> assertEquals("undefined", window.getMember("fontFaceHelper")); >> window.setMember("fontFaceHelper", fontFaceHelper); >> assertTrue(window.getMember("fontFaceHelper") instanceof >> FontFaceTestHelper); >> // Create font-face object from byte[] >> webView.getEngine().executeScript( >> "var byteArray = new >> Uint8Array(fontFaceHelper.ttfFileContent);\n" + >> "var arrayBuffer = byteArray.buffer;\n" + >> "window.fontFace1 = new FontFace('WebFont test', >> arrayBuffer, {});" >> ); >> >> webView.getEngine().executeScript("document.execCommand('selectAll', false, >> 'true');"); >> assertEquals("loaded", >> webView.getEngine().executeScript("fontFace1.status")); >> // Add font-face to Document, so that it could be usable on >> styles >> //webView.getEngine().executeScript( >> // "document.fonts.add(fontFace1);\n" + >> // >> "document.getElementById('probe1').style.fontFamily = 'WebFont test';\n" >> //); >> // webView.getEngine().getLoadWorker().stateProperty(). >> // addListener((observable, oldValue, newValue) -> { >> // if (newValue == SUCCEEDED) { >> // htmlEditor.requestFocus(); >> // } >> // }); >> >> assertNotNull(fontFamilyComboBox); >> assertFalse(fontFamilyComboBox.getItems().isEmpty()); >> String theChosenOne = null; >> for(String fontFamily: fontFamilyComboBox.getItems()){ >> if(fontFamily.contains(" ")){ >> theChosenOne = fontFamily; >> break; >> } >> } >> assertNotNull(theChosenOne); >> //Select the font >> fontFamilyComboBox.getSelectionModel().select(theChosenOne); >> assertEquals(theChosenOne, >> fontFamilyComboBox.getSelectionModel().getSelectedItem()); >> >> >> webView.getEngine().executeScript("document.execCommand('selectAll', false, >> 'true');"); >> >> assertEquals(theChosenOne, >> fontFamilyComboBox.getSelectionModel().getSelectedItem()); >> >> >> >> editorStateLatch.countDown(); >> }); >> >> assertTrue("Timeout when waiting for focus change ", >> Util.await(editorStateLatch)); >> } >> >> where fontFamilyComboBox is retrieved like that : >> fontFamilyComboBox = (ComboBox<String>) >> htmlEditor.lookup(".font-menu-button"); >> assertNotNull(fontFamilyComboBox); >> >> This idea is to add a Font family with a space in it, then apply it on the >> text. But I'm struggling to select a text portion, apply the font, then >> select elsewhere, and select back the text to demonstrate that the ComboBox >> is not updated. I tried to trigger some selectAll command but it's not >> working. >> >> If anyone has some ideas, I'm all ears. > > This bug still exists when the caret is placed on a text with the default > font (""). Applying this patch creates a new bug: Selecting text with multiple fonts in HTMLEditor sets the text to a single font. Steps to reproduce: Run the same sample program. Type "Hello world". Set "Hello" to FontA and "world" to FontB Select all (or right to left) using keyboard Expectation: "Hello world" is selected Observation: "Hello world" is selected and font changed to FontA PR: https://git.openjdk.java.net/jfx/pull/12
