JDK-8158926 : Char value is set as integer, not as character
  • Type: Bug
  • Component: javafx
  • Sub-Component: web
  • Affected Version: 8,9
  • Priority: P3
  • Status: Resolved
  • Resolution: Fixed
  • OS: generic
  • CPU: generic
  • Submitted: 2016-06-07
  • Updated: 2017-09-11
  • Resolved: 2016-06-24
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 8 JDK 9
8u112Fixed 9 masterFixed
Related Reports
Relates :  
Relates :  
Relates :  
Description
Reproducible: always 
Platform-specific: no
Steps to reproduce: same as in parent bug.
Steps to reproduce:
1. Set javascript char variable from java.
2. Try to read the same variable from java code.
Expected: java.lang.String with single character OR java.lang.Character with original value.
Actual: Numeric value of original character returned.
Sample code: 

import javafx.application.Application;
import javafx.scene.Scene;
import javafx.scene.layout.StackPane;
import javafx.scene.web.WebEngine;
import javafx.scene.web.WebView;
import javafx.stage.Stage;
import netscape.javascript.JSObject;

public class CharBindingFailure extends Application {
    @Override
    public void start(Stage stage) throws Exception {
        WebView view = new WebView();
        WebEngine engine = view.getEngine();
        stage.setScene(new Scene(new StackPane(view), 320, 240));
        stage.setOnShown((e) -> {
            char value = 'a';
            JSObject window = (JSObject) engine.executeScript("window;");
            window.setMember("testObject", value);
            Object result = engine.executeScript("testObject;");
            System.out.println(result.getClass());
        });
        stage.show();
    }
    public static void main(String[] args) {
        launch(args);
    }
} 

RULE "WebNodeAutomated/com/sun/fx/webnode/tests/bridge/javascript2java/AddRemoveCommonTypesTest/testAddRemoveJavaScriptBindingChar" Exception java.lang.AssertionError: expected:<a> but was:<97>
RULE "WebNodeAutomated/com/sun/fx/webnode/tests/bridge/javascript2java/MethodCallsTest/testCharMethodCallA" Exception java.lang.AssertionError: expected:<n> but was:<110>
RULE "WebNodeAutomated/com/sun/fx/webnode/tests/bridge/javascript2java/MethodCallsTest/testCharMethodCallA2" Exception java.lang.AssertionError: expected:<n> but was:<110>
Comments
Since it isn't a regression, a follow-on bug seems best.
27-06-2016

@Kevin, Just now realized that I missed few other case 1. "return as char" scenario. (e.g. executeScript("obj.getChar()") 2. Get char field of Java Object from JS (e.g. executeScript("obj.charMember")) Non of the above are regressions, but #1 is covered in WebNodeAutomatedTest. Two ways I could think of fixing it, 1. Backout the changset which I have pushed and create a new changeset which addresses new issue as well. 2. Create a incremental bug to address "return as char" scenario.
27-06-2016

changeset: 439e19883bc5 user: arajkumar date: Fri Jun 24 07:03:37 2016 +0100 8158926: Char value is set as integer, not as character Reviewed-by: ghb, kcr, mbilla URL: http://hg.openjdk.java.net/openjfx/9-dev/rt/rev/439e19883bc5
24-06-2016

+1
24-06-2016

+1
23-06-2016

In JavaInstanceJSC.cpp, regarding below change, // Since we can't convert java.lang.Character to any JS primitive, we have 282 // to handle valueOf method call. 283 jobject obj = m_instance->instance(); 284 JavaClass* aClass = static_cast<JavaClass*>(getClass()); 285 if (aClass && aClass->isCharacterClass() && jMethod->name() == "valueOf") 286 return numberValueForCharacter(obj); Can you keep above code after below code, since m_instance->instance(); is a WeakGlobalRef now and if obj is NULL, then numberValueForCharacter(obj) will return invalid value. // Since m_instance->instance() is WeakGlobalRef, creating a localref to safeguard instance() from GC 289 JLObject jlinstance(obj, true); 290 291 if (!jlinstance) { 292 LOG_ERROR("Could not get javaInstance for %p in JavaInstance::invokeMethod", jlinstance); 293 return jsUndefined(); 294 } Also you need to add check if aclass is NULL. then return jsUndefined() . JavaClass* aClass = static_cast<JavaClass*>(getClass()); if (!aClass) return jsUndefined();
23-06-2016

Updated the solution to maintain the Java type(Character) instead of converting it to String. Java Character will be considered as a foreign type in JS context and wrapped inside JavaInstance. http://cr.openjdk.java.net/~arajkumar/8158926/webrev.01
23-06-2016

Looks good +1
23-06-2016

http://cr.openjdk.java.net/~arajkumar/8158926/webrev.00
23-06-2016

looks fine.. +1
23-06-2016

Removed unnecessary null checks as pointed. http://cr.openjdk.java.net/~arajkumar/8158926/webrev.02
23-06-2016

>> Can you remove this check [ (if (!aClass) return jsUndefined(); ] at those 3 places as part of this fix?? Since these changes are trivial, I will combine as part of this fix.
23-06-2016

Right. in that case, you can remove NULL check. Also there are around 3 places , null check for getClass() is present and i remembered we didn't remove this check during memory leak fix. Can you remove this check [ (if (!aClass) return jsUndefined(); ] at those 3 places as part of this fix??
23-06-2016

>> 285 if (aClass && aClass->isCharacterClass() && jMethod->name() == "valueOf") >> 286 return numberValueForCharacter(obj); numberValueForCharacter handles WeakRef case. >> Also you need to add check if aclass is NULL. then return jsUndefined() . >> Class* JavaInstance::getClass() const >> { >> if (!m_class) { >> jobject acc = accessControlContext(); >> m_class = new JavaClass (m_instance->instance(), rootObject(), acc); >> } >> return m_class; >>} aClass won't be null according to the above code. I can remove the null check.
23-06-2016

It is completely different from the testcase which is attached with JDK-8089842. >> Object result = engine.executeScript("testObject='1';"); >> System.out.println(result.getClass()); If I change the test like above, "result.getClass()" returns java.lang.String. There is some issues with JSObject.setMember which need to be fixed.
21-06-2016

moved back to P3
20-06-2016

Not a regression in 8u102, since it happens in 8 as well.
20-06-2016

I can also confirm that the above test case fails before and after the fix for JDK-8089842. In looking at it, this is not a "fix fail" case, since the test case attached to JDK-8089842 now passes. Rather this bug is a different, but related problem. I recommend changing the summary of this bug to reflect this.
20-06-2016

Yes, checked with 9b123
20-06-2016

Andrey, does it affect 9 ?
20-06-2016

Resolve for 16_04.
20-06-2016

Andrey please reopen with more info how to reproduce it for Arun
13-06-2016

Tried on latest tree with the test case provided in JDK-8089842, couldn't reproduce.
10-06-2016

Original JDK-8089842 is P4, introduced in 8u40, and this is not a regression for 8u102
08-06-2016

Can you provide a standalone test case?
07-06-2016