JDK-8090011 : 'tab' key makes control loose focus
  • Type: Bug
  • Component: javafx
  • Sub-Component: web
  • Affected Version: 8,9
  • Priority: P4
  • Status: Closed
  • Resolution: Fixed
  • Submitted: 2013-10-10
  • Updated: 2020-01-31
  • Resolved: 2017-12-12
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
10Fixed 8u172Fixed
Related Reports
Cloners :  
Cloners :  
Relates :  
Description
To reproduce open HelloHTMLEditor, click on the text and push 'TAB'

Comments
There is a regression (JDK-8200418) due to this fix
02-05-2018

WebRev for backport: http://cr.openjdk.java.net/~rkamath/8090011/webrev.backport.00/
13-12-2017

+1 for Backport
13-12-2017

Changeset: 674513271a88 Author: rkamath Date: 2017-12-12 12:08 +0530 URL: http://hg.openjdk.java.net/openjfx/jfx-dev/rt/rev/674513271a88
12-12-2017

Looks good. +1
08-12-2017

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.
24-11-2017