Hello,

I strongly recommend using

    String.strip

rather than

    String.trim

as Strim.trim uses a idosyncratic definition of whitespace whereas the new-in-11 String.strip uses a conventional definition of white space. Likewise, I recommend linking "white space" to the definition of white space used by Character.isWhitespace.

Thanks,

-Joe

On 3/7/2019 10:56 PM, Shashidhara Veerabhadraiah wrote:

Thank you Phil. Sure will wait till CSR is approved.

Thanks and regards,

shashi

*From:*Philip Race
*Sent:* Friday, March 8, 2019 12:18 PM
*To:* Shashidhara Veerabhadraiah <shashidhara.veerabhadra...@oracle.com>
*Cc:* awt-dev@openjdk.java.net; Joe Darcy <joe.da...@oracle.com>; swing-...@openjdk.java.net *Subject:* Re: <AWT Dev> <Swing Dev> [13] JDK-8216008: -Djavax.accessibility.assistive_technologies empty list leads to crash

Ok but you need to wait for the CSR to be approved before you can push.

-phil.

On 3/8/19, 11:41 AM, Shashidhara Veerabhadraiah wrote:

    Hi Phil, As discussed here are the updates for the Webrev.
    Hopefully this is correct.

    http://cr.openjdk.java.net/~sveerabhadra/8216008/webrev.09/
    <http://cr.openjdk.java.net/%7Esveerabhadra/8216008/webrev.09/>

    Thanks and regards,

    Shashi

    *From:*Shashidhara Veerabhadraiah
    *Sent:* Friday, March 8, 2019 10:33 AM
    *To:* Philip Race <philip.r...@oracle.com>
    <mailto:philip.r...@oracle.com>
    *Cc:* awt-dev@openjdk.java.net <mailto:awt-dev@openjdk.java.net>;
    Joe Darcy <joe.da...@oracle.com> <mailto:joe.da...@oracle.com>;
    swing-...@openjdk.java.net <mailto:swing-...@openjdk.java.net>
    *Subject:* Re: <AWT Dev> <Swing Dev> [13] JDK-8216008:
    -Djavax.accessibility.assistive_technologies empty list leads to crash

    Thank you Phil for the review and here are the updates done:

    http://cr.openjdk.java.net/~sveerabhadra/8216008//webrev.08/
    <http://cr.openjdk.java.net/%7Esveerabhadra/8216008/webrev.08/>

    Thanks and regards,

    Shashi

    *From:*Philip Race
    *Sent:* Wednesday, March 6, 2019 10:42 PM
    *To:* Shashidhara Veerabhadraiah
    <shashidhara.veerabhadra...@oracle.com
    <mailto:shashidhara.veerabhadra...@oracle.com>>
    *Cc:* awt-dev@openjdk.java.net <mailto:awt-dev@openjdk.java.net>;
    swing-...@openjdk.java.net <mailto:swing-...@openjdk.java.net>;
    Joe Darcy <joe.da...@oracle.com <mailto:joe.da...@oracle.com>>
    *Subject:* Re: <AWT Dev> <Swing Dev> [13] JDK-8216008:
    -Djavax.accessibility.assistive_technologies empty list leads to crash

    the webrev needs a couple of minor tweaks :

    contains empty string  -> is the empty string

    and

    immeadiately -> immediately


    Go ahead and finalize the CSR which looks OK.

    -phil.

    On 3/6/19, 11:45 AM, Shashidhara Veerabhadraiah wrote:

        Hi Phil\Sergey, Please review the CSR along with the bug fix.

        Webrev:
        http://cr.openjdk.java.net/~sveerabhadra/8216008/webrev.07/
        <http://cr.openjdk.java.net/%7Esveerabhadra/8216008/webrev.07/>

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

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

        Thanks and regards,

        Shashi

        *From:*Shashidhara Veerabhadraiah
        *Sent:* Monday, March 4, 2019 3:36 PM
        *To:* Philip Race <philip.r...@oracle.com>
        <mailto:philip.r...@oracle.com>; Joe Darcy
        <joe.da...@oracle.com> <mailto:joe.da...@oracle.com>
        *Cc:* awt-dev@openjdk.java.net
        <mailto:awt-dev@openjdk.java.net>; swing-...@openjdk.java.net
        <mailto:swing-...@openjdk.java.net>
        *Subject:* Re: <AWT Dev> <Swing Dev> [13] JDK-8216008:
        -Djavax.accessibility.assistive_technologies empty list leads
        to crash

        Hi Joe, Please find the updated Webrev fixing the typo:

        http://cr.openjdk.java.net/~sveerabhadra/8216008/webrev.07/
        <http://cr.openjdk.java.net/%7Esveerabhadra/8216008/webrev.07/>

        Thanks and regards,

        Shashi

        *From:*Shashidhara Veerabhadraiah
        *Sent:* Thursday, February 21, 2019 2:08 PM
        *To:* Philip Race <philip.r...@oracle.com
        <mailto:philip.r...@oracle.com>>; Joe Darcy
        <joe.da...@oracle.com <mailto:joe.da...@oracle.com>>
        *Cc:* awt-dev@openjdk.java.net
        <mailto:awt-dev@openjdk.java.net>; swing-...@openjdk.java.net
        <mailto:swing-...@openjdk.java.net>
        *Subject:* Re: <AWT Dev> <Swing Dev> [13] JDK-8216008:
        -Djavax.accessibility.assistive_technologies empty list leads
        to crash

        Hi Joe, Please find the new Webrev fixing the specification as
        asked in the CSR bug:
        https://bugs.openjdk.java.net/browse/JDK-8218737

        Webrev:
        http://cr.openjdk.java.net/~sveerabhadra/8216008/webrev.06/
        <http://cr.openjdk.java.net/%7Esveerabhadra/8216008/webrev.06/>

        Thanks and regards,

        Shashi

        *From:*Shashidhara Veerabhadraiah
        *Sent:* Monday, February 18, 2019 10:19 AM
        *To:* Philip Race <philip.r...@oracle.com
        <mailto:philip.r...@oracle.com>>
        *Cc:* awt-dev@openjdk.java.net
        <mailto:awt-dev@openjdk.java.net>; swing-...@openjdk.java.net
        <mailto:swing-...@openjdk.java.net>
        *Subject:* Re: <AWT Dev> <Swing Dev> [13] JDK-8216008:
        -Djavax.accessibility.assistive_technologies empty list leads
        to crash

        Thank you Phil for that help on CSR.

        *From:*Phil Race
        *Sent:* Friday, February 15, 2019 9:41 PM
        *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

        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/
            <http://cr.openjdk.java.net/%7Esveerabhadra/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>
            <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

            +     * 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/
                <http://cr.openjdk.java.net/%7Esveerabhadra/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/  
<http://cr.openjdk.java.net/%7Esveerabhadra/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/  
<http://cr.openjdk.java.net/%7Esveerabhadra/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/  
<http://cr.openjdk.java.net/%7Esveerabhadra/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