Hello Sergey, Thanks for the review comments. Please take a look at the updated webrev: http://cr.openjdk.java.net/~arapte/a11y/4949105/webrev.02/
Changes as per the review comments: 1. I have checked some of our code, and it looks like we sometimes set ACCESSIBLE_NAME_PROPERTY explicitly to some value. Is it possible that the fix will broke that? > Yes, the previous change looks like breaking this scenario. > ACCESSIBLE_NAME_PROPERTY could get overwritten with the previous change. 2. Probably we need to update this property only if the old value of this property is aligned to the old BasicHTML.propertyKey value? > This sounds better option. > The new change will update the ACCESSIBLE_NAME_PROPERTY from > BasicHTML.updateRenderer() only if, a. If AccessibleContext.ACCESSIBLE_NAME_PROPERTY was NOT set before b. If AccessibleContext.ACCESSIBLE_NAME_PROPERTY was previously set from this method using the View.getDocument().getText(). 3. BTW why we call getText(1,len-1) instead of getText(0,len) as in all others places? > This is because on the windows 7 machine with JAWS 2018, the > View.getDocument().getText() returned a string with leading and trailing new > line character. But on Windows 10 and with JAWS 2019, , the View.getDocument().getText() returns string with only leading new line character. I do not have Win7 machine anymore, so won't be able to reverify the behavior, Anyway changed code to use String.trim() method. 4. Added new test to verify the scenario in point 2. Regards, Ambarish -----Original Message----- From: Sergey Bylokhov Sent: Monday, December 2, 2019 10:38 AM To: Ambarish Rapte <[email protected]>; [email protected] Subject: Re: <Swing Dev> [14] RFR : JDK-4949105 : Access Bridge lacks html tags parsing Hi, Ambarish. I have checked some of our code, and it looks like we sometimes set ACCESSIBLE_NAME_PROPERTY explicitly to some value. Is it possible that the fix will broke that? Probably we need to update this property only if the old value of this property is aligned to the old BasicHTML.propertyKey value? BTW why we call getText(1,len-1) instead of getText(0,len) as in all others places? On 10/31/19 4:57 am, Ambarish Rapte wrote: > Hi Sergey, > Thanks for the review, > Here is the updated webrev: > http://cr.openjdk.java.net/~arapte/a11y/4949105/webrev.01/ > > Fixed both the review comments: > 1. AccessibleContext.ACCESSIBLE_NAME_PROPERTY shall be set only if the text > is html string, otherwise it is set to null which is existing behavior. > 2. Removed printStackTrace() call. In case of exception > AccessibleContext.ACCESSIBLE_NAME_PROPERTY shall be set to null which is > existing behavior. > 3. Added a test for setText() scenario. > > > Regards, > Ambarish > > -----Original Message----- > From: Sergey Bylokhov > Sent: Thursday, October 31, 2019 3:25 AM > To: Ambarish Rapte <[email protected]>; > [email protected] > Subject: Re: <Swing Dev> [14] RFR : JDK-4949105 : Access Bridge lacks > html tags parsing > > Hi, Ambarish. > > I have checked behavior change, and it looks like it has some unexpected > issues. > > If the text of some component like JLabel sets the text to HTML and then to > the simple text, then HTML based a11y name will be used instead of simple > text. > > Note that you should not use "printStackTrace" I guess the old behavior > should be used if some errors occur. > > On 9/24/19 9:45 am, Ambarish Rapte wrote: >> Hi All, >> >> Please review this accessibility fix, >> >> JBS: https://bugs.openjdk.java.net/browse/JDK-4949105 >> >> Webrev: http://cr.openjdk.java.net/~arapte/a11y/4949105/webrev.00/ >> >> Issue: >> >> When a Swing component is created with html text. The same html text is >> shared as AccessibleName with screen reader application. >> >> If screen reader fails to parse the html string correctly, then it may read >> the text different from the text displayed for the component. >> >> Fix: >> >> The same parsed html string for the component should be shared to screen >> reader as AccessibleName. >> >> No existing Swing component tests fail due to this change. >> >> Regards, >> >> Ambarish >> > > > -- > Best regards, Sergey. > -- Best regards, Sergey.
