That text may actually be worse.
Although I maybe didn't get your distinction of null vs the empty string here ?

Oh, wait. Why is this doc clarification on the PRIVATE implementation method ? It needs to go on the public getDefaultToolkit() method to have any point whatsoever
especially from a CSR perspective.
Try this :
-     * toolkit is created. All errors are handled via an AWTError exception.
+     * toolkit is created.
+     * If the list of assistive technology providers is the empty string, or +     * contains only white space characters then the method returns immediately.
+     * All other errors are handled via an AWTError exception.


I think the CSR needs some work too. I've made some updates there.

-phil.

On 2/15/19 2:10 AM, Shashidhara Veerabhadraiah wrote:

Hi Phil, I have updated the CSR and updated the information for the function and here is the new Webrev:

http://cr.openjdk.java.net/~sveerabhadra/8216008/webrev.05/

Thanks and regards,

Shashi

*From:*Phil Race
*Sent:* Friday, February 15, 2019 2:58 AM
*To:* Shashidhara Veerabhadraiah <shashidhara.veerabhadra...@oracle.com>
*Cc:* Sergey Bylokhov <sergey.bylok...@oracle.com>; awt-dev@openjdk.java.net; swing-...@openjdk.java.net *Subject:* Re: <Swing Dev> [13] JDK-8216008: -Djavax.accessibility.assistive_technologies empty list leads to crash

+     * activate method.  If the list of assistive technology providers is empty string,
is "the" empty string.
The CSR needs to be updated to include this spec.
What is there now in the specification section needs to be fixed
It should not point to the webrev or review and right now
doesn't contain the change to the javadoc in the body of the CSR
I think I even have to question most of the rest of it.
Any one reading it would think that we always used to throw an exception in such case and now want to stop doing so. Isn't the bug that we did NOT throw an exception and now we do ? You only hint at that when you say at the end of "Solution", "as was the case earlier"
If I am right the only thing you need in the CSR is to say that you
are reverting the implementation to previous behaviour and updating the
specification to make it clear that this behaviour is allowed by the spec
since this case was unclear.
-phil.

On 2/12/19 1:08 AM, Shashidhara Veerabhadraiah wrote:

    Hi Phil, Here is the new Webrev and CSR for the same:

    http://cr.openjdk.java.net/~sveerabhadra/8216008/webrev.04/

    CSR: https://bugs.openjdk.java.net/browse/JDK-8218737

    Thanks and regards,

    Shashi

    *From:*Philip Race
    *Sent:* Saturday, February 9, 2019 2:18 AM
    *To:* Shashidhara Veerabhadraiah
    <shashidhara.veerabhadra...@oracle.com>
    <mailto:shashidhara.veerabhadra...@oracle.com>
    *Cc:* Sergey Bylokhov <sergey.bylok...@oracle.com>
    <mailto:sergey.bylok...@oracle.com>; awt-dev@openjdk.java.net
    <mailto:awt-dev@openjdk.java.net>; swing-...@openjdk.java.net
    <mailto:swing-...@openjdk.java.net>
    *Subject:* Re: <Swing Dev> [13] JDK-8216008:
    -Djavax.accessibility.assistive_technologies empty list leads to crash

    FWIW I think this could be closed out as "not a bug".
    An empty value string is an error and the spec. says AWTError is
    how errors are reported. In fact I'll argue that 8 was wrong not
    to have thrown an exception in such a case.
    It was an accident of the implementation that it did not.
    But if we want to be behaviourally compatible then ...
    Note that since you now document this you need an approved CSR
    BEFORE pushing it.

    And if you go the CSR route you may need to be more precise about what
    you mean by "empty".

    I think it needs to say

    "If the list of assistive technology providers is null, or
    contains only white
    space characters then the method returns immediately.
    All other errors are handled by throwing {@code AWTError}"

    @throws AWTError if there is any error in parsing or loading the ATs.

    -phil.

    On 1/21/19, 9:12 AM, Shashidhara Veerabhadraiah wrote:

        Hi Sergey, Here is the new Webrev for your comments:

        http://cr.openjdk.java.net/~sveerabhadra/8216008/webrev.02/

        Thanks and regards,

        Shashi

        -----Original Message-----

        From: Sergey Bylokhov

        Sent: Saturday, January 19, 2019 3:47 AM

        To: Shashidhara Veerabhadraiah<shashidhara.veerabhadra...@oracle.com>  
<mailto:shashidhara.veerabhadra...@oracle.com>;awt-dev@openjdk.java.net  
<mailto:awt-dev@openjdk.java.net>;swing-...@openjdk.java.net  
<mailto:swing-...@openjdk.java.net>

        Subject: Re: <Swing Dev> [13] JDK-8216008: 
-Djavax.accessibility.assistive_technologies empty list leads to crash

        On 16/01/2019 23:03,shashidhara.veerabhadra...@oracle.com  
<mailto:shashidhara.veerabhadra...@oracle.com>  wrote:

            Another point is that the whitespace trimming should not trigger additional input 
processing  for the custom class that will be used for assistive technology. For eg., if custom 
class "FooProvider" is implemented and if we pass "  FooProvider  " by mistake, 
a bug may be created to trim the whitespace in this case as well!!

        Yes, my previous email suggested to always trim the content of the "input", it will cover 
all cases, the "atNames" is empty, the "atNames" contains only the whitespace, or the 
name of the class has some spaces at the start/end.

            Thanks and regards,

            Shashi

            On 17/01/19 10:28 AM, Shashidhara Veerabhadraiah wrote:

                We need one way to tell the system not to load any assistive 
technologies and which is being provided.

                If we add trailing white spaces removal then we may need to add 
another functionality to see if that class exists or not and based on that not 
to load anything(without throwing the error because we are parsing the input 
content). I think that would take a different direction. I feel it is fair 
enough to provide one way to tell not to load any assistive technologies and 
additional parsing would only lead to other ways/expectations.

                Thanks and regards,

                Shashi

                -----Original Message-----

                From: Sergey Bylokhov

                Sent: Thursday, January 17, 2019 1:33 AM

                To: Shashidhara Veerabhadraiah<shashidhara.veerabhadra...@oracle.com>  
<mailto:shashidhara.veerabhadra...@oracle.com>;awt-dev@openjdk.java.net  
<mailto:awt-dev@openjdk.java.net>;swing-...@openjdk.java.net  
<mailto:swing-...@openjdk.java.net>

                Subject: Re: <Swing Dev> [13] JDK-8216008: 
-Djavax.accessibility.assistive_technologies empty list leads to crash

                I guess you meant whitespaces, then it is unclear why we should not consider 
"empty" parameter as a "mistake"?

                On 15/01/2019 20:37, Shashidhara Veerabhadraiah wrote:

                    Hi Sergey, I felt that a string with an empty space is 
intentional and should be considering it as a 'mistake' if done by the 
developers. Hence I feel it is not required(as there is a way to tell nothing 
to load). Please let me know if you think otherwise.

                    Thanks and regards,

                    Shashi

                    -----Original Message-----

                    From: Sergey Bylokhov

                    Sent: Tuesday, January 15, 2019 7:32 AM

                    To: Shashidhara Veerabhadraiah<shashidhara.veerabhadra...@oracle.com>  
<mailto:shashidhara.veerabhadra...@oracle.com>;awt-dev@openjdk.java.net  
<mailto:awt-dev@openjdk.java.net>;swing-...@openjdk.java.net  
<mailto:swing-...@openjdk.java.net>

                    Subject: Re: <Swing Dev> [13] JDK-8216008: 
-Djavax.accessibility.assistive_technologies empty list leads to crash

                    Hi, Shashi.

                    Should we trim all whitespaces from the "atNames"?

                    Otherwise the "atNames", which contains only one 
whitespace, will trigger the same error.

                    On 09/01/2019 00:20, Shashidhara Veerabhadraiah wrote:

                        Hi Sergey, Thanks for pointing that out. Here is the 
new Webrev:

                        
http://cr.openjdk.java.net/~sveerabhadra/8216008/webrev.01/

                        Thanks and regards,

                        Shashi

                        -----Original Message-----

                        From: Sergey Bylokhov

                        Sent: Friday, January 4, 2019 3:48 AM

                        To: Shashidhara Veerabhadraiah<shashidhara.veerabhadra...@oracle.com>  
<mailto:shashidhara.veerabhadra...@oracle.com>;awt-dev@openjdk.java.net  
<mailto:awt-dev@openjdk.java.net>;swing-...@openjdk.java.net  
<mailto:swing-...@openjdk.java.net>

                        Subject: Re: <Swing Dev> [13] JDK-8216008: 
-Djavax.accessibility.assistive_technologies empty list leads to crash

                        Hi, Shashi.

                        I think you can update an existing test:

                        
open/test/jdk/javax/accessibility/AccessibilityProvider/basic.sh

                        On 03/01/2019 01:36, Shashidhara Veerabhadraiah wrote:

                            Hi All, Please review a fix for the below bug.

                            Bug:https://bugs.openjdk.java.net/browse/JDK-8216008

                            
Webrev:http://cr.openjdk.java.net/~sveerabhadra/8216008/webrev.00/

                            Since the mentioned list of assistive technologies 
for the system property(javax.accessibility.assistive_technologies) is loaded 
by jvm am unable to write a test case for this bug. The test file attached with 
the bug can be used for testing this fix.

                            This fix removes the error(class not found error) 
by passing an empty list to system variable 
javax.accessibility.assistive_technologies and does not load any assistive 
technologies(as the list is empty). Currently empty list produce a crash which 
is not required as there was no class name mentioned in the list. Please read 
the bug comments for more information.

                            Thanks and regards,

                            Shashi


Reply via email to