Looks good to me. Regards, Manajit
> > From: Krishna Addepalli <krishna.addepa...@oracle.com > <mailto:krishna.addepa...@oracle.com>> > Subject: Re: <Swing Dev> [11][JDK-8195095]Images are not scaled correctly in > JEditorPane > Date: 7 March 2018 at 9:48:44 PM IST > To: Semyon Sadetsky <semyon.sadet...@oracle.com > <mailto:semyon.sadet...@oracle.com>>, Prasanta Sadhukhan > <prasanta.sadhuk...@oracle.com <mailto:prasanta.sadhuk...@oracle.com>>, > swing-dev@openjdk.java.net <mailto:swing-dev@openjdk.java.net> > > > Hi Semyon, > > To reduce the possibility of the test passing / failing due to wrong reasons, > I have changed the color of the image to blue, and updated the test > accordingly. > Here is the new webrev: > http://cr.openjdk.java.net/~kaddepalli/8195095/webrev02 > <http://cr.openjdk.java.net/~kaddepalli/8195095/webrev02> > > Thanks, > Krishna > <> > From: Krishna Addepalli > Sent: Monday, March 5, 2018 11:59 PM > To: Semyon Sadetsky <semyon.sadet...@oracle.com > <mailto:semyon.sadet...@oracle.com>>; Prasanta Sadhukhan > <prasanta.sadhuk...@oracle.com <mailto:prasanta.sadhuk...@oracle.com>>; > swing-dev@openjdk.java.net <mailto:swing-dev@openjdk.java.net> > Subject: RE: <Swing Dev> [11][JDK-8195095]Images are not scaled correctly in > JEditorPane > > I tried on Ubuntu 17.10, but since the getPixelColor always returns black, > the test will pass, which is why I was asking. > > Thanks, > Krishna > > From: Semyon Sadetsky > Sent: Monday, March 5, 2018 11:57 PM > To: Krishna Addepalli <krishna.addepa...@oracle.com > <mailto:krishna.addepa...@oracle.com>>; Prasanta Sadhukhan > <prasanta.sadhuk...@oracle.com <mailto:prasanta.sadhuk...@oracle.com>>; > swing-dev@openjdk.java.net <mailto:swing-dev@openjdk.java.net> > Subject: Re: <Swing Dev> [11][JDK-8195095]Images are not scaled correctly in > JEditorPane > > On 03/05/2018 10:23 AM, Krishna Addepalli wrote: > > Hi Semyon, > > Did you try it in Windows or Linux? I have tested it on Mac, and found that > it fails before the fix and passes after the fix. > I tried it on Linux. But the change is in generic code why the test result > depends on the platform? > > --Semyon > > > Thanks, > Krishna > > From: Semyon Sadetsky > Sent: Monday, March 5, 2018 10:01 PM > To: Krishna Addepalli <krishna.addepa...@oracle.com> > <mailto:krishna.addepa...@oracle.com>; Prasanta Sadhukhan > <prasanta.sadhuk...@oracle.com> <mailto:prasanta.sadhuk...@oracle.com>; > swing-dev@openjdk.java.net <mailto:swing-dev@openjdk.java.net> > Subject: Re: <Swing Dev> [11][JDK-8195095]Images are not scaled correctly in > JEditorPane > > Hi Krishna, > > I tried your test before the fix and it passed. > > --Semyon > > > On 03/03/2018 12:07 AM, Krishna Addepalli wrote: > Hi Prasanta, > > Thanks for the quick review. I have updated the testcase with your > suggestion. Here it is: > http://cr.openjdk.java.net/~kaddepalli/8195095/webrev01 > <http://cr.openjdk.java.net/%7Ekaddepalli/8195095/webrev01> > > Krishna > > From: Prasanta Sadhukhan > Sent: Saturday, March 3, 2018 1:06 PM > To: Krishna Addepalli <krishna.addepa...@oracle.com> > <mailto:krishna.addepa...@oracle.com>; swing-dev@openjdk.java.net > <mailto:swing-dev@openjdk.java.net> > Subject: Re: <Swing Dev> [11][JDK-8195095]Images are not scaled correctly in > JEditorPane > > looks good to me. > > Only thing, in test, JFrame creation also needs to be in EDT. > > Regards > Prasanta > On 3/3/2018 12:40 PM, Krishna Addepalli wrote: > Hi All, > > Please review a fix for JDK-8195095: > https://bugs.openjdk.java.net/browse/JDK-8195095 > <https://bugs.openjdk.java.net/browse/JDK-8195095> > > Webrev: http://cr.openjdk.java.net/~kaddepalli/8195095/webrev00 > <http://cr.openjdk.java.net/%7Ekaddepalli/8195095/webrev00> > > Problem: When a text html is specified with an image, omitting either the > height/width parameter, JEditorPane scales it to default value of the missing > parameter, whereas it should fetch the actual value from the image. > > Fix: The problem is in ImageView.java. In the imageUpdate function, in the > resizing logic the “changed” flag is set to 1 if height is present, and to 2 > if width is present. But while updating the width, the check is swapped, i.e > width is set if flag is 1 and vice versa. > > PS: The webrev contains a new “circle.png” file, so when you import the > patch, you may need to manually copy the file, since “hg import” doesnot > automatically import the .png files from the patch file. > > Thanks > Krishna > > > > > > > > On 07/03/2018 08:19, Krishna Addepalli wrote: >> Thanks for the review Semyon! >> >> -----Original Message----- >> From: Semyon Sadetsky >> Sent: Wednesday, March 7, 2018 9:27 PM >> To: Krishna Addepalli <krishna.addepa...@oracle.com >> <mailto:krishna.addepa...@oracle.com>>; Philip Race <philip.r...@oracle.com >> <mailto:philip.r...@oracle.com>> >> Cc: swing-dev@openjdk.java.net <mailto:swing-dev@openjdk.java.net> >> Subject: Re: <Swing Dev> >> [11][JDK-8197785]javax.accessibility.AccessibilityBundle will reload the >> ResourceBundle for every call to toDisplayString >> >> +1 >> >> --Semyon >> >> >> On 3/7/18 5:28 AM, Krishna Addepalli wrote: >>> Hi Semyon, >>> >>> Modified the code as per your suggestions. Here is the new webrev: >>> http://cr.openjdk.java.net/~kaddepalli/8197785/webrev02 >>> <http://cr.openjdk.java.net/~kaddepalli/8197785/webrev02> >>> >>> Thanks, >>> Krishna >>> >>> -----Original Message----- >>> From: Semyon Sadetsky >>> Sent: Tuesday, March 6, 2018 11:46 PM >>> To: Phil Race <philip.r...@oracle.com <mailto:philip.r...@oracle.com>>; >>> Krishna Addepalli >>> <krishna.addepa...@oracle.com <mailto:krishna.addepa...@oracle.com>> >>> Cc: swing-dev@openjdk.java.net <mailto:swing-dev@openjdk.java.net> >>> Subject: Re: <Swing Dev> >>> [11][JDK-8197785]javax.accessibility.AccessibilityBundle will reload >>> the ResourceBundle for every call to toDisplayString >>> >>> On 03/06/2018 10:01 AM, Phil Race wrote: >>> >>>> >>>> On 03/06/2018 09:45 AM, Semyon Sadetsky wrote: >>>>> Can you point to place where this Hashmap is updated other then >>>>> where it is initialized? >>>> You mean Hashtable ? >>> Right, sorry. >>>> 161 table.put(locale, resourceTable); >>>> >>>> Will be executed once per locale. >>> It updates "table" not "resourceTable". The "table" field could be treated >>> differently since writing in it really very rare but it is ok to leave it >>> as it is. All what I meant concerned only the "resourceTable". >>> >>> --Semyon >>>> -phil. >>>> >> > > > -- > Best regards, Sergey. > > > > > >> Hi, Krishna. >> The .02 version of the fix have changed the behavior of >> toDisplayString() method. Before the fix it throws NPE for key=null, >> after the fix it returns null. > That's ok. It is an improvement. NPE is not expected result according to > the toDisplayString() method spec. > > --Semyon > >> I guess version .01 should be fine as it simple and supposed to fix >> one reported bug. >> >> On 07/03/2018 08:19, Krishna Addepalli wrote: >>> Thanks for the review Semyon! >>> >>> -----Original Message----- >>> From: Semyon Sadetsky >>> Sent: Wednesday, March 7, 2018 9:27 PM >>> To: Krishna Addepalli <krishna.addepa...@oracle.com >>> <mailto:krishna.addepa...@oracle.com>>; Philip Race >>> <philip.r...@oracle.com <mailto:philip.r...@oracle.com>> >>> Cc: swing-dev@openjdk.java.net <mailto:swing-dev@openjdk.java.net> >>> Subject: Re: <Swing Dev> >>> [11][JDK-8197785]javax.accessibility.AccessibilityBundle will reload >>> the ResourceBundle for every call to toDisplayString >>> >>> +1 >>> >>> --Semyon >>> >>> >>> On 3/7/18 5:28 AM, Krishna Addepalli wrote: >>>> Hi Semyon, >>>> >>>> Modified the code as per your suggestions. Here is the new webrev: >>>> http://cr.openjdk.java.net/~kaddepalli/8197785/webrev02 >>>> <http://cr.openjdk.java.net/~kaddepalli/8197785/webrev02> >>>> >>>> Thanks, >>>> Krishna >>>> >>>> -----Original Message----- >>>> From: Semyon Sadetsky >>>> Sent: Tuesday, March 6, 2018 11:46 PM >>>> To: Phil Race <philip.r...@oracle.com <mailto:philip.r...@oracle.com>>; >>>> Krishna Addepalli >>>> <krishna.addepa...@oracle.com <mailto:krishna.addepa...@oracle.com>> >>>> Cc: swing-dev@openjdk.java.net <mailto:swing-dev@openjdk.java.net> >>>> Subject: Re: <Swing Dev> >>>> [11][JDK-8197785]javax.accessibility.AccessibilityBundle will reload >>>> the ResourceBundle for every call to toDisplayString >>>> >>>> On 03/06/2018 10:01 AM, Phil Race wrote: >>>> >>>>> >>>>> On 03/06/2018 09:45 AM, Semyon Sadetsky wrote: >>>>>> Can you point to place where this Hashmap is updated other then >>>>>> where it is initialized? >>>>> You mean Hashtable ? >>>> Right, sorry. >>>>> 161 table.put(locale, resourceTable); >>>>> >>>>> Will be executed once per locale. >>>> It updates "table" not "resourceTable". The "table" field could be >>>> treated differently since writing in it really very rare but it is >>>> ok to leave it as it is. All what I meant concerned only the >>>> "resourceTable". >>>> >>>> --Semyon >>>>> -phil. >>>>> >>> >> >> > > > >
