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.
RDP1 is approaching (Dec 14) it means P4s will be re-targeted to "11" by Dec 14.
http://mail.openjdk.java.net/pipermail/jdk-dev/2017-December/000357.html
08-12-2017
+1
Please note that while backporting to 8, the statement "package test.javafx.scene.web;" should be "package javafx.scene.web;"
08-12-2017
Thanks all for the review.
-Removed shim and used node lookup as per [~arajkumar] suggestion.
-Incorporated other comments given by [~ghb] and [~mbilla]
PTAL at updated webrev:
http://cr.openjdk.java.net/~rkamath/8090011/webrev.05/
07-12-2017
lgtm +1
nit:
Can you use Node.lookup to get embedded WebView instance of HTMLEditor? just to avoid HTMLEditorShim.
WebView webView = (WebView) htmlEditor.lookup(".web-view");
07-12-2017
Minor comments:
1) You can remove un-used imports like import com.sun.javafx.application.PlatformImpl;
2) Remove one extra space before "one" in // Maintain one application instance
07-12-2017
+1 with nit
a. add space in assertTrue
b. use pre increment in for loop with note for 10 tabs (fire events)
07-12-2017
Moved the test case to system tests. Request all to please review
http://cr.openjdk.java.net/~rkamath/8090011/webrev.04/
Please Note:
Not sure if this is a known issue but when i run system tests using "gradle -PCOMPILE_WEBKIT=true -PFULL_TEST=true :systemTests:test" these tests are consistently failing with\without my patch
----Linux[ Ubuntu 16.04 VM]---
PNTMeshVertexBufferLengthTest. testMeshWithFiveDiv
PNTMeshVertexBufferLengthTest. testMeshWithOneDiv
PNTMeshVertexBufferLengthTest. testMeshWithThreeDiv
PNTMeshVertexBufferLengthTest. testMeshWithTwoDiv
PNTMeshVertexBufferLengthTest. testMeshWithZeroDiv
Warning in standard error for these => "WARNING: System can't support ConditionalFeature.SCENE3D"
---Windows---
NewSceneSizeTest. testNewSceneSize
RestoreSceneSizeTest. testUnfullscreenSize
The above errors seem to be precision related
With the patch i have verified that nothing else is failing apart from the above.
07-12-2017
Thanks [~kcr] and [~mbilla] for the review.
I referred the controls module wherein show was getting called and thought we could as well.
Thanks for the clarification. Will move this to system tests.
05-12-2017
[~rkamath] Though test case looks fine, But not sure about having stage.show() and stage.close() in unit test case.
The only unit test case which create stage is ScreenAndWindowTest.java and does not call stage.show and stage.close
05-12-2017
OK, I took a look at the test, and this is not suitable for the javafx.web module. No other test in that module calls Stage::show. Other than the two projects that use the StubToolkit (graphics and controls), tests that need to show a stage should use the systemTests project.
The fact that you have to call a static method like Platform::setImplicitExit is another red flag that this test needs to run in systemTests where each test file runs in its own JVM.
05-12-2017
Added regression test in this webrev. Verified TC before and after fix.
Request to please review =>
http://cr.openjdk.java.net/~rkamath/8090011/webrev.03/
05-12-2017
Addressed #2 in this webrev
http://cr.openjdk.java.net/~rkamath/8090011/webrev.02/
With respect to #1,
Had started with that approach, but it is not possible to test HTMLEditor directly with the limited exposed APIs that it has. In this case, focus is still within HTMLEditor control but WebKit focus has changed. So we would need access to the internal web fragments to test. So, in this test case, a rich text editor is emulated to check behavior with and without design mode which i thought would help test the generic root cause.
But yes, you are right that the test does not check the specific HTMLEditor fix. Would try to approach this again by adding required helper APIs.
01-12-2017
[~rkamath]
1. As per my understanding, the test case will check the behavior with DesignMode disable and Enable.
So current test case will always pass without / with your fix (unless some functionality breaks for DesignMode). Im ok with test case, if we dont have any other way to check for regression (like test case should fail without fix)
2. Below minor comment about coding guidelines (need to give space before /after operator )
for (int i=0; i<10; i++) {
change to
for (int i = 0; i < 10; i++) {
01-12-2017
Thanks [~rkamath]. looks good to me.
01-12-2017
What i observed is that the 'tab' characters are inserted with the help of execCommand("insertTab") in HTMLEditor by listening to key events.
If we just have a editable body in a webview and keypress 'tab' it does not insert the 'tab' spacing.(only focus is determined)
01-12-2017
Can you also check whether the body contents has `tab` characters?
01-12-2017
Added test case. Please review
http://cr.openjdk.java.net/~rkamath/8090011/webrev.01/
29-11-2017
+1
28-11-2017
Thanks [~arajkumar]
Let me try for a regression test.
27-11-2017
WebKit tab eventhandler switches the focus between frames on pressing the tab key. This is fine when it comes to normal controls. But in case of Rich Text Editors, we would expect tab to insert the "TAB" spacing and focus be retained. Currently, HTMLEditor is inserting the "TAB" spacing but the focus is lost because of event handling.
FX HTMLEditor only sets the contenteditable property to body element. However, when the entire document is editable(like in case of RTE), WebKit designmode needs to be set as well. Setting this will make WebKit tab eventhandler escape the focus switch.
Please review the proposed fix:
http://cr.openjdk.java.net/~rkamath/8090011/webrev.00/
27-11-2017
lgtm.
Is it possible to write a regression test?
27-11-2017
Have looked into this issue and figured out the root cause.
The behavior is primarily because of focus getting transferred in the tab event handler inside WebKit event handling.
Testing a tentative fix for this. Will update further.