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 for additional webrev, obviously.


Regards,
Alexey

On 08.06.2016 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 following code:

registerEditorKitForContentType("test/test", "com.oracle.Test",
<non-null-customized-classLoader>);

registerEditorKitForContentType("test/test", "com.sun.Test", null);

If the classloader is not removed from KitLoaderRegistry, then class
"com.sun.Test" would be loaded using the wrong class loader which was
set for class "com.oracle.Test".

Thanks for clarification! Then I suggest to update the test to be sure that this use-case works properly.



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 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 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/8158734/9/webrev.00/

A simple null check was added.

Thanks,
Mikhail.














Reply via email to