Re: [9] Review request for 8158734 JEditorPane.createEditorKitForContentType throws NPE after 6882559

2016-06-09 Thread mikhail cherkasov
On 6/9/2016 12:40 PM, Alexey Ivanov wrote: Except for: 44 static public class MyEditorKit extends EditorKit { Could you please use blessed keyword order: public static? sure, I'll fix it. And in the error message: "Class loader was not been removed for '"… Remove “been” as it's out of

Re: [9] Review request for 8158734 JEditorPane.createEditorKitForContentType throws NPE after 6882559

2016-06-09 Thread Alexey Ivanov
Hi Mikhail, Looks fine. Except for: 44 static public class MyEditorKit extends EditorKit { Could you please use blessed keyword order: public static? And in the error message: "Class loader was not been removed for '"… Remove “been” as it's out of its place in this sentence. No need fo

Re: [9] Review request for 8158734 JEditorPane.createEditorKitForContentType throws NPE after 6882559

2016-06-09 Thread Sergey Bylokhov
Looks fine. On 08.06.16 20:23, mikhail cherkasov wrote: The testcase was modified to check that we removes classloader: http://cr.openjdk.java.net/~mcherkas/8158734/9/webrev.04/ On 6/8/2016 6:00 PM, Sergey Bylokhov wrote: On 08.06.16 17:58, Alexey Ivanov wrote: I think it should. Consider the

Re: [9] Review request for 8158734 JEditorPane.createEditorKitForContentType throws NPE after 6882559

2016-06-08 Thread mikhail cherkasov
The testcase was modified to check that we removes classloader: http://cr.openjdk.java.net/~mcherkas/8158734/9/webrev.04/ On 6/8/2016 6:00 PM, Sergey Bylokhov wrote: On 08.06.16 17:58, Alexey Ivanov wrote: I think it should. Consider the following code: registerEditorKitForContentType("test/te

Re: [9] Review request for 8158734 JEditorPane.createEditorKitForContentType throws NPE after 6882559

2016-06-08 Thread Sergey Bylokhov
On 08.06.16 17:58, Alexey Ivanov wrote: I think it should. Consider the following code: registerEditorKitForContentType("test/test", "com.oracle.Test", ); registerEditorKitForContentType("test/test", "com.sun.Test", null); If the classloader is not removed from KitLoaderRegistry, then class "c

Re: [9] Review request for 8158734 JEditorPane.createEditorKitForContentType throws NPE after 6882559

2016-06-08 Thread Alexey Ivanov
Hi Mikhail, Sergey, On 08.06.2016 17:43, Sergey Bylokhov wrote: On 08.06.16 17:31, mikhail cherkasov wrote: And one more change: cr.openjdk.java.net/~mcherkas/8158734/9/webrev.02 I added a removing of old class loader if a new is null. Why it should be removed? I am not sure but is the null c

Re: [9] Review request for 8158734 JEditorPane.createEditorKitForContentType throws NPE after 6882559

2016-06-08 Thread mikhail cherkasov
On 6/8/2016 5:43 PM, Sergey Bylokhov wrote: Why it should be removed? I am not sure but is the null classloader is a bootclassloader? Yes, null class loader is a bootstrap class loader. I think if someone will pass null as classloader it expect to remove an old one and use bootclassloader, it

Re: [9] Review request for 8158734 JEditorPane.createEditorKitForContentType throws NPE after 6882559

2016-06-08 Thread Sergey Bylokhov
On 08.06.16 17:31, mikhail cherkasov wrote: And one more change: cr.openjdk.java.net/~mcherkas/8158734/9/webrev.02 I added a removing of old class loader if a new is null. Why it should be removed? I am not sure but is the null classloader is a bootclassloader? On 6/8/2016 3:16 PM, Alexand

Re: [9] Review request for 8158734 JEditorPane.createEditorKitForContentType throws NPE after 6882559

2016-06-08 Thread mikhail cherkasov
And one more change: cr.openjdk.java.net/~mcherkas/8158734/9/webrev.02 I added a removing of old class loader if a new is null. On 6/8/2016 3:16 PM, Alexander Potochkin wrote: Hello Mikhail This version looks good to me Thanks alexp Hi Sergey, you are right, it's easier to check classloader

Re: [9] Review request for 8158734 JEditorPane.createEditorKitForContentType throws NPE after 6882559

2016-06-08 Thread Alexander Potochkin
Hello Mikhail This version looks good to me Thanks alexp Hi Sergey, you are right, it's easier to check classloader for null before to add it to hashtable: http://cr.openjdk.java.net/~mcherkas/8158734/9/webrev.01/src/java.desktop/share/classes/javax/swing/JEditorPane.java.udiff.html On 6

Re: [9] Review request for 8158734 JEditorPane.createEditorKitForContentType throws NPE after 6882559

2016-06-07 Thread mikhail cherkasov
Hi Sergey, you are right, it's easier to check classloader for null before to add it to hashtable: http://cr.openjdk.java.net/~mcherkas/8158734/9/webrev.01/src/java.desktop/share/classes/javax/swing/JEditorPane.java.udiff.html On 6/7/2016 10:51 PM, Sergey Bylokhov wrote: HI, Mikhail. Can you

Re: [9] Review request for 8158734 JEditorPane.createEditorKitForContentType throws NPE after 6882559

2016-06-07 Thread Sergey Bylokhov
HI, Mikhail. Can you please clarify the reason of usage Optional here if the optional itself can be null? On 07.06.16 22:05, mikhail cherkasov wrote: Hello, Could you please review the fix for: https://bugs.openjdk.java.net/browse/JDK-8158734 webrev: http://cr.openjdk.java.net/~mcherkas/8158

[9] Review request for 8158734 JEditorPane.createEditorKitForContentType throws NPE after 6882559

2016-06-07 Thread mikhail cherkasov
Hello, Could you please review the fix for: https://bugs.openjdk.java.net/browse/JDK-8158734 webrev: http://cr.openjdk.java.net/~mcherkas/8158734/9/webrev.00/ A simple null check was added. Thanks, Mikhail.