JDK-8218737 : -Djavax.accessibility.assistive_technologies empty list leads to exception
  • Type: CSR
  • Component: client-libs
  • Sub-Component: javax.accessibility
  • Priority: P3
  • Status: Closed
  • Resolution: Approved
  • Fix Versions: 13
  • Submitted: 2019-02-11
  • Updated: 2019-04-16
  • Resolved: 2019-04-16
Related Reports
CSR :  
Description
Summary
-------
Make clear in the specification of java.awt.Toolkit the handling of empty or white space values for
the system property "javax.accessibility.assistive_technologies".


Problem
-------
The system property javax.accessibility.assistive_technologies has been used since JDK 1.2 to
name a class, or comma-separated list of classes, which load Assistive Technologies.
The specification has always documented AWTError is thrown when there is any problem
either parsing the value, or loading the classes.
An empty string does not specify a valid class name, but prior to JDK 9, as an artifact of the
parsing implementation, this produced no error.
In JDK 9, changes which were made as part of migrating to support the module system
unconsciously changed this behavior to throw AWTError.


Solution
--------
The proposed solution is to restore compatibility with long-standing behavior and
document this handling in the specification to make it clear.


Specification
-------------
    src/java.desktop/share/classes/java/awt/Toolkit.java

    .......
    -     * toolkit is created. All errors are handled via an AWTError exception.
    +     * toolkit is created.
    +     * <p>
    +     * If the list of assistive technology providers as provided through system
    +     * property "{@systemProperty javax.accessibility.assistive_technologies}"
    +     * is the empty string or contains only
    +     * {@linkplain Character#isWhitespace(int) white space} characters it is ignored.
    +     * All other errors are handled via an AWTError exception.
    .......
    public static synchronized Toolkit getDefaultToolkit() {

Reference for convenience: http://cr.openjdk.java.net/~serb/8216008/webrev.00/src/java.desktop/share/classes/java/awt/Toolkit.java.udiff.html
Comments
Moving revised request to Approved.
16-04-2019

The text as written implies all-whitespace is an error condition, because it is an error which is handled differently than any other possible errors.
16-04-2019

The text as written implies all-whitespace is an error condition ("All other errors...") so I don't think assuming the AWT toolkit is returned is the only reasonable interpretation. I'd recommend either saying something like "If {@code javax.accessibility.assistive_technologies} is the empty string or contains only {@linkplain Character#isWhitespace(int) white space} characters it is ignored. Otherwise.." or "...method returns the AWT Toolkit." HTH
16-04-2019

I guess it is clear since the text above is followed immediately after "Service providers are loaded after the AWT toolkit is created." So the current spec: <pre> /** * Gets the default toolkit. ........ * If this Toolkit is not a headless implementation and if they exist, service * providers of {@link javax.accessibility.AccessibilityProvider} will be loaded * if specified by the system property * {@code javax.accessibility.assistive_technologies}. ..... * using a comma separated list. Service providers are loaded after the AWT - * toolkit is created. All errors are handled via an AWTError exception. + * toolkit is created. + * If the list of assistive technology providers as provided through system + * property "{@systemProperty javax.accessibility.assistive_technologies}" + * is the empty string or contains only + * {@linkplain Character#isWhitespace(int) white space} characters then the + * method returns immediately. All other errors are handled via an AWTError + * exception. .... * @return the default toolkit. </pre>
16-04-2019

Okay -- perhaps 620 return null; is for the anonymous class. In any case, rephrasing my feedback, in 563 * {@linkplain Character#isWhitespace(int) white space} characters then the 564 * method returns immediately. All other errors are handled via an AWTError 565 * exception. I'd like to see on line 564 "the method returns XYZ immediately" where XYZ describes the return value. I don't think the behavior in this case is clear from local context.
16-04-2019

I guess you are looking to the private method in the webrev, the getDefaultToolkit() method in the description of this CSR always return java.awt.Toolkit.
16-04-2019

Please update the spec to explicitly state what value is returned if the properly only has whitespace characters is it. Looking at the implementing, the method appears to return null in some cases and a null return value doesn't seem to be described by the spec.
16-04-2019

To make sure expectations are explicit, this issue will stay in the pended state until it is updated by the requestors. There is not action on the part of the CSR to get it out of the pended state until the request is updated for re-review.
27-03-2019

To clarify the state of this this request, when I ask in a CSR review "Please change/fix/clarify X and Y" what I mean is "Please change/fix/clarify X *and* Y". If X and Y are not both addressed when the request is resubmitted for review, it will *not* be approved. If those working on the request do not think addressing either X or Y is necessary, that should be discussed as part of the review thread.
18-03-2019

To be more explicit, to indicate which of several possible white space definitions are being used replace * contains only white space characters then the method returns immediately. with * contains only {@linkplain Character#isWhitespace(int) white space} characters then the method returns immediately. Also, as previously requested please document the system property using the {@systemProperty} javadoc tag (JDK-8213920). Re-pending the request.
13-03-2019

New webrev fixing above commments: http://cr.openjdk.java.net/~sveerabhadra/8216008/webrev.10/
12-03-2019

As noted in a code review comment, please use a conventional definition of white space, such as Character.isWhite, which implies the implementation should use String.strip rather than String.trim. Also, please document the system property using the {@systemProperty} javadoc tag (JDK-8213920). Pending the request while this is worked out.
08-03-2019

Final webrev: http://cr.openjdk.java.net/~sveerabhadra/8216008//webrev.08/
08-03-2019

Re-pending the request. To proceed please: 1) Get a reviewer on the request 2) Hit "Edit" and correct the interface kind as requested twice before. 3) "Finalize" the request. It is small enough to not go through the two-phase CSR process.
05-03-2019

Please find the fix @ http://cr.openjdk.java.net/~sveerabhadra/8216008/webrev.07/
04-03-2019

The interface kind does not appear to be set correctly. A Java API is being at least nominally modified as is the handling of a system property. System properties can now be documented, see JDK-5076751. This CSR does not have a reviewer. Re-pending the request until the requested corrections and additions are made. The linked-to webrev contains a typo "technolog": 555 * using a comma separated list. Service providers are loaded after the AWT 556 * toolkit is created. 557 * If the list of assistive technolog providers is the empty string, or 558 * contains only white space characters then the method returns immeadiately. 559 * All other errors are handled via an AWTError exception. 560 * <p>
27-02-2019

Please find the updated webrev: http://cr.openjdk.java.net/~sveerabhadra/8216008//webrev.06/
21-02-2019

Requested updates have not been made (no reviewer, etc.); pending the request.
21-02-2019

Before this request is finalized, besides getting a reviewer, please clarify what exact interfaces are being changed or proposed. In particular, it appears that a new system property is being defined or modified. Moving to Provisional.
15-02-2019