Hi Prasanta, Yes, you are right. But, does it make a difference if we still update the values even if they are same?
Thanks, Krishna > On 04-Mar-2019, at 5:12 PM, Prasanta Sadhukhan > <prasanta.sadhuk...@oracle.com> wrote: > > > > On 04-Mar-19 4:53 PM, Krishna Addepalli wrote: >> Hi Prasanta, >> >> Thanks for the review. Here is the updated webrev: >> http://cr.openjdk.java.net/~kaddepalli/8218674/webrev01/ >> <http://cr.openjdk.java.net/%7Ekaddepalli/8218674/webrev01/> >> >> The check (newWidth>0) && (newHeight > 0) is done in adjustWidthHeight >> function. >> > if (specifiedWidth != -1 && specifiedHeight != -1) { > 915 newWidth = specifiedWidth; > 916 newHeight = specifiedHeight; > 917 } > ... > 931 d.width = newWidth; > 932 d.height = newHeight; > > If specifiedWidth/Height is 0, then I do not see any check, it is just > assigning to d.width/height, no? In that case, can we need to change the > newstate without checking? > > Regards > Prasanta >> Thanks, >> Krishna >> >>> On 04-Mar-2019, at 1:29 PM, Prasanta Sadhukhan >>> <prasanta.sadhuk...@oracle.com <mailto:prasanta.sadhuk...@oracle.com>> >>> wrote: >>> >>> Hi Krishna, >>> You can reuse existing method getLoadsSynchronously() instead of checking >>> for (state & SYNC_LOAD_FLAG) != 0) >>> >>> BTW, do we not have to check if (newWidth > 0) & newHeight >0 before >>> changing the newstate @l797? >>> >>> The copyright year in testcase should be changed to 2019. >>> >>> Regards >>> Prasanta >>> On 01-Mar-19 12:10 PM, Krishna Addepalli wrote: >>>> Thanks for the review Sergey. >>>> Can I have one more review? Prasanta maybe? >>>> >>>> Thanks, >>>> Krishna >>>> >>>>> On 01-Mar-2019, at 3:40 AM, Sergey Bylokhov <sergey.bylok...@oracle.com> >>>>> <mailto:sergey.bylok...@oracle.com> wrote: >>>>> >>>>> Looks fine. >>>>> >>>>> On 21/02/2019 08:44, Sergey Bylokhov wrote: >>>>>> On 20/02/2019 22:21, Krishna Addepalli wrote: >>>>>>> Hi Sergey, >>>>>>> >>>>>>> I have fixed the issue. Could you check now? >>>>>> Yes, it works now, I look to the fix. >>>>>>> Thanks, >>>>>>> Krishna >>>>>>> -----Original Message----- >>>>>>> From: Sergey Bylokhov >>>>>>> Sent: Thursday, February 21, 2019 3:54 AM >>>>>>> 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> [13] RFR: JDK-8218674 - HTML Tooltip with >>>>>>> "img=src" on component doesn't show >>>>>>> >>>>>>> Hi, Krishna. >>>>>>> >>>>>>> Some links have wrong file permissions, "403 - Forbidden": >>>>>>> http://cr.openjdk.java.net/~kaddepalli/8218674/webrev00/raw_files/new/test/jdk/javax/swing/text/html/8218674/circle.png >>>>>>> >>>>>>> <http://cr.openjdk.java.net/%7Ekaddepalli/8218674/webrev00/raw_files/new/test/jdk/javax/swing/text/html/8218674/circle.png> >>>>>>> http://cr.openjdk.java.net/~kaddepalli/8218674/webrev00/raw_files/new/test/jdk/javax/swing/text/html/8218674/TooltipImageTest.java >>>>>>> >>>>>>> <http://cr.openjdk.java.net/%7Ekaddepalli/8218674/webrev00/raw_files/new/test/jdk/javax/swing/text/html/8218674/TooltipImageTest.java> >>>>>>> >>>>>>> On 20/02/2019 03:57, Krishna Addepalli wrote: >>>>>>>> Hi All, >>>>>>>> >>>>>>>> Please review a fix for the bug JDK-8218674: >>>>>>>> https://bugs.openjdk.java.net/browse/JDK-8218674 >>>>>>>> <https://bugs.openjdk.java.net/browse/JDK-8218674> >>>>>>>> Webrev: http://cr.openjdk.java.net/~kaddepalli/8218674/webrev00/ >>>>>>>> <http://cr.openjdk.java.net/%7Ekaddepalli/8218674/webrev00/> >>>>>>>> >>>>>>>> This is a regression introduced due to fix for JDK-8208638. The >>>>>>>> default behaviour for ImageView is to load an image asynchronously. >>>>>>>> Hence, it uses the ImageHandler::imageUpdate to get the updates to the >>>>>>>> image being loaded. That will set the width and height of the image >>>>>>>> view. ImageView::updateImageSize does not alter the width and height >>>>>>>> in this case. When a JToolTip is created and html text set as tooltip, >>>>>>>> internally, the image is requested to be loaded synchronously, and in >>>>>>>> this case, ImageView::updateImageSize is the only way to calculate the >>>>>>>> image size. Since the width and height were not specified in the >>>>>>>> tooltip, the image was not being drawn. >>>>>>>> The fix is to check if the image is requested to be loaded >>>>>>>> synchronously, and if so, then do the same calculation as for the fix >>>>>>>> for JDK-8208638, which will provide valid image width and height, >>>>>>>> additionally also taking care of the scaling issues fixed for >>>>>>>> JDK-8208638. >>>>>>>> I have tested the fix on Windows, Linux(Ubuntu) and Mac, and found >>>>>>>> that it is working. I have also run all the jtreg tests under the >>>>>>>> test/jdk/javax/swing/text/html, and found no new failures. >>>>>>>> >>>>>>>> Thanks, >>>>>>>> Krishna >>>>> -- >>>>> Best regards, Sergey. >>> >> >
