Hello Sergey,

The fix looks good to me in general, but I have a few comments:

It would be nice to add @Override annotation to java.awt.Window.adjustDescendantsOnParent()

--- old/src/java.desktop/share/classes/java/awt/Component.java  2014-09-10 
15:11:22.601342000 +0400
+++ new/src/java.desktop/share/classes/java/awt/Component.java  2014-09-10 
15:11:21.809727900 +0400
@@ -781,11 +781,12 @@
          */
         OTHER
     }
/*
-     * The shape set with the applyCompoundShape() method. It uncludes the 
result
+     * The shape set with the applyCompoundShape() method. It includes the
+     * result
      * of the HW/LW mixing related shape computation. It may also include
      * the user-specified shape of the component.
      * The 'null' value means the component has normal shape (or has no shape 
at all)
      * and applyCompoundShape() will skip the following shape identical to 
normal.
      */

I think that result should not be at the next line. (80 chars per line limit already violated a few lines below
so changing only one line does not make any sense to me).


java/awt/Component.java "bootstrap class loader" vs java/awt/GraphicsEnvironment.java "bootstrap classloader"
I think that we should choose and use only one title.


This may be irrelevant to this issue, but after grepping sources for "Scirpt" usage I've found similar typo in CSS.java: ./java/awt/font/NumericShaper.java:92: * EnumSet.of(NumericShaper.Scirpt.ARABIC, NumericShaper.Range.TAMIL) ./javax/swing/text/html/CSS.java:879: * If the vertical alignment is set to either superscirpt or


BTW,  I like the idea of advertisement in source code :) :
java/awt/image/ColorModel.java
            // algorithm for linear RGB to nonlinear sRGB conversion
            // is from the IEC 61966-2-1 International Standard,
            // Colour Management - Default RGB colour space - sRGB,
            // First Edition, 1999-10,
            // available for order at http://www.iec.ch

Thanks,

Alexander.

On 09/10/2014 04:11 PM, Sergey Bylokhov wrote:
Hi, Phil.
It seems both changes are unnecessary:
http://cr.openjdk.java.net/~serb/8055326/webrev.01

On 21.08.2014 22:04, Phil Race wrote:
Was the additional spce on the 2nd line intended here ?

--- old/src/java.desktop/share/classes/java/awt/ContainerOrderFocusTraversalPolicy.java 2014-08-21 20:50:04.859532400 +0400 +++ new/src/java.desktop/share/classes/java/awt/ContainerOrderFocusTraversalPolicy.java 2014-08-21 20:50:04.663521200 +0400
@@ -166,8 +166,8 @@
retComp = cont.getFocusTraversalPolicy().getDefaultComponent(cont);

if (retComp != null && log.isLoggable(PlatformLogger.Level.FINE)) { - log.fine("### Transfered focus down-cycle to " + retComp +
-                                 " in the focus cycle root " + cont);
+ log.fine("### Transferred focus down-cycle to " + retComp +
+                                " in the focus cycle root " + cont);


And I don't see what was so wrong with this, perhaps because I wrote it :-)

-         * so that when the Font2D is GC'd it can also remove the file.
+ * so that when the Font2D is GC'ed it can also remove the file.


Other than that, looks good.
Some fun new words in there. I particularly liked utilitized and unclude.

-phil.

On 8/21/2014 10:38 AM, Sergey Bylokhov wrote:
Hello,
Please review the fix for jdk 9.
The fix was contributed by  pavel.ra...@oracle.com

Bug: https://bugs.openjdk.java.net/browse/JDK-8055326
Webrev can be found at: http://cr.openjdk.java.net/~serb/8055326/webrev.00





Reply via email to