Re: [13] RFR JDK-8212904:JTextArea line wrapping incorrect when using UI scale
Hi Sergey, Thanks for your review. textarea.getLineCount() returns same number for both correct and broken wrap so it cannot be used for test automation... Regards Prasanta On 21-Mar-19 4:24 AM, Sergey Bylokhov wrote: Hi, Prasanta. The change looks fine, but probably the test can be automated? I guess when "wrap" works properly you should get more lines in the text area, than in case of broken "wrap"? On 19/03/2019 00:03, Prasanta Sadhukhan wrote: Hi All, Please review a fix for an issue where it is seen that line wrapping of JTextArea doen't work correctly if you set wrapStyleWord = true and you use a UI scale (either by setting "sun.java2d.uiScale" or by setting display scale of Windows) It is a regression of JDK-8132119: Provide public API for text related methods in SwingUtilities2 where many public text related API was added catering to floating point hidpi scale. This issue happened because of that as the public Utilities.getBreakLocation() API, used to determine where to break the given text, is using non-floating point methods. Proposed fix is to make sure correct getBreakLocation() method is called and not the deprecated(integer based) one and also the floating point based getBreakLocation() API uses floating point API based calculations. Bug: https://bugs.openjdk.java.net/browse/JDK-8212904 webrev: http://cr.openjdk.java.net/~psadhukhan/8212904/webrev.0/ Regards Prasanta
Re: [13] RFR JDK-8220250: fix headings in java.desktop
I have not changed any build files. I only modified CompileJavaModules to see if it builds fine as was asked for. Regards Prasanta On 21-Mar-19 8:33 PM, Jonathan Gibbons wrote: Sounds good; note you should cc: build-dev@ojn for changes to build files. -- Jon On 3/21/19 3:23 AM, Prasanta Sadhukhan wrote: FYI...it does build fine with this build change. Regards Prasanta On 19-Mar-19 10:58 PM, Jonathan Gibbons wrote: Prasanta, Note that if you believe you have fixed all the heading issues in the java.desktop module, you can re-enable the doclint checks for the module by removing "-accessibility" from the java.desktop line in make/CompileJavaModules.gmk make/CompileJavaModules.gmk:java.desktop_ADD_JAVAC_FLAGS += -Xdoclint:all/protected,-reference,-accessibility \ If you do remove the option, make sure to run a build to verify that doclint does not find any unfixed errors, which will prevent the build from completing successfully. -- Jon On 3/19/19 3:41 AM, Prasanta Sadhukhan wrote: Hi Alexey, Your points are valid so I modified those in this webrev, please have a look http://cr.openjdk.java.net/~psadhukhan/8220250/webrev.3/ Regards Prasanta On 15-Mar-19 10:28 PM, Alexey Ivanov wrote: Hi Prasanta, On 15/03/2019 05:15, Prasanta Sadhukhan wrote: Hi Alexey, On 13-Mar-19 6:10 PM, Alexey Ivanov wrote: Hi guys, This is more of a general comment rather than targeted at this particular change. Sorry if I missed any previous discussion on that matter. I think it's good to have consistent headings without skipping levels. Yet in the Javadoc comments does not seem right. Let's take a look at JTable for example: http://cr.openjdk.java.net/~psadhukhan/8220250/docs.webrev2/docs/api/java.desktop/javax/swing/JTable.html The page starts with JTable. This is the highest heading level on the page, I haven't found any . As for this changeset, I am following the rules set in the https://mail.openjdk.java.net/pipermail/jdk-dev/2019-March/002671.html - headings in documentation comments for modules, packages and types should start at - Thank you for clarification. I've seen a few fixes in this area but I missed this discussion. So the main type heading will be , i.e. it will JTable. Therefore starting at in types makes sense. Later on, Method Detail. And a specific method: doLayout. Since the entire method description is under heading, it should not use any headings above . But now we have and which kind-of start new sections rather than being part of which describes the method. I have only changed to make sure there are no missing headings ie start with h2 (as stated above) and change previously used h4 to h3(to make sure no missing heading), rest were as it was earlier. According to the message you linked to, the documentation comments for methods should start at . Here's the relevant quote: …Headings in documentation comments for modules, packages and types should start at ; Headings in documentation comments for members of a type (field, constructor, method, etc) should start at . (I've changed formatting to separate these two cases.) Thus in *JTable.java* you should rather decrease heading level as the headings are used in /method/ as opposed to /type/ in the majority of other cases: -3053 * Distributing the delta +3053 * Distributing the delta -3055 * Overview +3055 * Overview And the remaining become : 3064 * Definition 3096 * Details 3107 * When the MAX and MIN bounds are hit As for the rest of the review: *Font.java* You changed the first to : -77 * Characters and Glyphs +77 * Characters and Glyphs But why not other headings? All of the headings were at the same level, but they're not. These should also go up to : 99 * Physical and Logical Fonts 138 * Font Faces and Names 172 * Font and TextAttribute *javax/print/attribute/package-info.java* 204 * Attribute Class Design 298 * Attribute Vendors 324 * Using Attributes These should also go one level up to as other headings. These used to be separate sections but now they're subsections of 131 * Attribute Sets which does not seem correct. Regards, Alexey Regards Prasanta Is such heading sequence intended? Regards, Alexey On 13/03/2019 06:10, Prasanta Sadhukhan wrote: Any more comments on this http://cr.openjdk.java.net/~psadhukhan/8220250/webrev.2/ Regards Prasanta On 11-Mar-19 2:16 PM, Prasanta Sadhukhan wrote: The 2nd URL is based on webrev.1. I have placed new docs for webrev.2 here http://cr.openjdk.java.net/~psadhukhan/8220250/docs.webrev2/docs/api/java.desktop/module-summary.html Regards Prasanta
Re: [13] Review Request: 8177960 Deprecate the Swing Motif Look and Feel and document it as unsupported on macOS
Yes, please push this - the CSR is approved -phil On 3/4/19, 8:25 PM, Prasanta Sadhukhan wrote: Looks good to me. Regards Prasanta On 05-Mar-19 6:17 AM, Sergey Bylokhov wrote: Hello. Please review the fix for JDK 13. Bug: https://bugs.openjdk.java.net/browse/JDK-8177960 CSR: https://bugs.openjdk.java.net/browse/JDK-8218637 Fix: http://cr.openjdk.java.net/~serb/8177960/webrev.01 This is a request to deprecate the Swing Motif Look and Feel, and document it as unsupported on macOS platform in JDK 13. The Motif L&F has long been part of the Swing implementation shipped with the JDK, although unlike L&Fs such as Metal and Nimbus it is not exposed in Java APIs and not part of the Java SE specification. This look and feel was implemented as the System look and feel for the Solaris platform at time when the CDE/Motif environment was used the principal supported desktop environment. But it was placed in shared code and provided and supported on all platforms including Windows and later macOS. Since it is superseded as a system L&F by the GTK L&F and has limited use and additionally as a L&F is very dated visually, we would like to discourage further use. Note in some future release we may remove it completely, or we may remove it from the shared code to stop distributing it on Windows and macOS. These are mentioned here to give the most possible notice of such possible additional changes.
Re: [13] RFR JDK-8220250: fix headings in java.desktop
Sounds good; note you should cc: build-dev@ojn for changes to build files. -- Jon On 3/21/19 3:23 AM, Prasanta Sadhukhan wrote: FYI...it does build fine with this build change. Regards Prasanta On 19-Mar-19 10:58 PM, Jonathan Gibbons wrote: Prasanta, Note that if you believe you have fixed all the heading issues in the java.desktop module, you can re-enable the doclint checks for the module by removing "-accessibility" from the java.desktop line in make/CompileJavaModules.gmk make/CompileJavaModules.gmk:java.desktop_ADD_JAVAC_FLAGS += -Xdoclint:all/protected,-reference,-accessibility \ If you do remove the option, make sure to run a build to verify that doclint does not find any unfixed errors, which will prevent the build from completing successfully. -- Jon On 3/19/19 3:41 AM, Prasanta Sadhukhan wrote: Hi Alexey, Your points are valid so I modified those in this webrev, please have a look http://cr.openjdk.java.net/~psadhukhan/8220250/webrev.3/ Regards Prasanta On 15-Mar-19 10:28 PM, Alexey Ivanov wrote: Hi Prasanta, On 15/03/2019 05:15, Prasanta Sadhukhan wrote: Hi Alexey, On 13-Mar-19 6:10 PM, Alexey Ivanov wrote: Hi guys, This is more of a general comment rather than targeted at this particular change. Sorry if I missed any previous discussion on that matter. I think it's good to have consistent headings without skipping levels. Yet in the Javadoc comments does not seem right. Let's take a look at JTable for example: http://cr.openjdk.java.net/~psadhukhan/8220250/docs.webrev2/docs/api/java.desktop/javax/swing/JTable.html The page starts with JTable. This is the highest heading level on the page, I haven't found any . As for this changeset, I am following the rules set in the https://mail.openjdk.java.net/pipermail/jdk-dev/2019-March/002671.html - headings in documentation comments for modules, packages and types should start at - Thank you for clarification. I've seen a few fixes in this area but I missed this discussion. So the main type heading will be , i.e. it will JTable. Therefore starting at in types makes sense. Later on, Method Detail. And a specific method: doLayout. Since the entire method description is under heading, it should not use any headings above . But now we have and which kind-of start new sections rather than being part of which describes the method. I have only changed to make sure there are no missing headings ie start with h2 (as stated above) and change previously used h4 to h3(to make sure no missing heading), rest were as it was earlier. According to the message you linked to, the documentation comments for methods should start at . Here's the relevant quote: …Headings in documentation comments for modules, packages and types should start at ; Headings in documentation comments for members of a type (field, constructor, method, etc) should start at . (I've changed formatting to separate these two cases.) Thus in *JTable.java* you should rather decrease heading level as the headings are used in /method/ as opposed to /type/ in the majority of other cases: -3053 * Distributing the delta +3053 * Distributing the delta -3055 * Overview +3055 * Overview And the remaining become : 3064 * Definition 3096 * Details 3107 * When the MAX and MIN bounds are hit As for the rest of the review: *Font.java* You changed the first to : -77 * Characters and Glyphs +77 * Characters and Glyphs But why not other headings? All of the headings were at the same level, but they're not. These should also go up to : 99 * Physical and Logical Fonts 138 * Font Faces and Names 172 * Font and TextAttribute *javax/print/attribute/package-info.java* 204 * Attribute Class Design 298 * Attribute Vendors 324 * Using Attributes These should also go one level up to as other headings. These used to be separate sections but now they're subsections of 131 * Attribute Sets which does not seem correct. Regards, Alexey Regards Prasanta Is such heading sequence intended? Regards, Alexey On 13/03/2019 06:10, Prasanta Sadhukhan wrote: Any more comments on this http://cr.openjdk.java.net/~psadhukhan/8220250/webrev.2/ Regards Prasanta On 11-Mar-19 2:16 PM, Prasanta Sadhukhan wrote: The 2nd URL is based on webrev.1. I have placed new docs for webrev.2 here http://cr.openjdk.java.net/~psadhukhan/8220250/docs.webrev2/docs/api/java.desktop/module-summary.html Regards Prasanta
Re: [13] RFR JDK-8220250: fix headings in java.desktop
FYI...it does build fine with this build change. Regards Prasanta On 19-Mar-19 10:58 PM, Jonathan Gibbons wrote: Prasanta, Note that if you believe you have fixed all the heading issues in the java.desktop module, you can re-enable the doclint checks for the module by removing "-accessibility" from the java.desktop line in make/CompileJavaModules.gmk make/CompileJavaModules.gmk:java.desktop_ADD_JAVAC_FLAGS += -Xdoclint:all/protected,-reference,-accessibility \ If you do remove the option, make sure to run a build to verify that doclint does not find any unfixed errors, which will prevent the build from completing successfully. -- Jon On 3/19/19 3:41 AM, Prasanta Sadhukhan wrote: Hi Alexey, Your points are valid so I modified those in this webrev, please have a look http://cr.openjdk.java.net/~psadhukhan/8220250/webrev.3/ Regards Prasanta On 15-Mar-19 10:28 PM, Alexey Ivanov wrote: Hi Prasanta, On 15/03/2019 05:15, Prasanta Sadhukhan wrote: Hi Alexey, On 13-Mar-19 6:10 PM, Alexey Ivanov wrote: Hi guys, This is more of a general comment rather than targeted at this particular change. Sorry if I missed any previous discussion on that matter. I think it's good to have consistent headings without skipping levels. Yet in the Javadoc comments does not seem right. Let's take a look at JTable for example: http://cr.openjdk.java.net/~psadhukhan/8220250/docs.webrev2/docs/api/java.desktop/javax/swing/JTable.html The page starts with JTable. This is the highest heading level on the page, I haven't found any . As for this changeset, I am following the rules set in the https://mail.openjdk.java.net/pipermail/jdk-dev/2019-March/002671.html - headings in documentation comments for modules, packages and types should start at - Thank you for clarification. I've seen a few fixes in this area but I missed this discussion. So the main type heading will be , i.e. it will JTable. Therefore starting at in types makes sense. Later on, Method Detail. And a specific method: doLayout. Since the entire method description is under heading, it should not use any headings above . But now we have and which kind-of start new sections rather than being part of which describes the method. I have only changed to make sure there are no missing headings ie start with h2 (as stated above) and change previously used h4 to h3(to make sure no missing heading), rest were as it was earlier. According to the message you linked to, the documentation comments for methods should start at . Here's the relevant quote: …Headings in documentation comments for modules, packages and types should start at ; Headings in documentation comments for members of a type (field, constructor, method, etc) should start at . (I've changed formatting to separate these two cases.) Thus in *JTable.java* you should rather decrease heading level as the headings are used in /method/ as opposed to /type/ in the majority of other cases: -3053 * Distributing the delta +3053 * Distributing the delta -3055 * Overview +3055 * Overview And the remaining become : 3064 * Definition 3096 * Details 3107 * When the MAX and MIN bounds are hit As for the rest of the review: *Font.java* You changed the first to : -77 * Characters and Glyphs +77 * Characters and Glyphs But why not other headings? All of the headings were at the same level, but they're not. These should also go up to : 99 * Physical and Logical Fonts 138 * Font Faces and Names 172 * Font and TextAttribute *javax/print/attribute/package-info.java* 204 * Attribute Class Design 298 * Attribute Vendors 324 * Using Attributes These should also go one level up to as other headings. These used to be separate sections but now they're subsections of 131 * Attribute Sets which does not seem correct. Regards, Alexey Regards Prasanta Is such heading sequence intended? Regards, Alexey On 13/03/2019 06:10, Prasanta Sadhukhan wrote: Any more comments on this http://cr.openjdk.java.net/~psadhukhan/8220250/webrev.2/ Regards Prasanta On 11-Mar-19 2:16 PM, Prasanta Sadhukhan wrote: The 2nd URL is based on webrev.1. I have placed new docs for webrev.2 here http://cr.openjdk.java.net/~psadhukhan/8220250/docs.webrev2/docs/api/java.desktop/module-summary.html Regards Prasanta
