Hi Prasanta, Thanks for the review. Here is the updated webrev: http://cr.openjdk.java.net/~kaddepalli/8218674/webrev01/ <http://cr.openjdk.java.net/~kaddepalli/8218674/webrev01/>
The check (newWidth>0) && (newHeight > 0) is done in adjustWidthHeight function. Thanks, Krishna > On 04-Mar-2019, at 1:29 PM, Prasanta Sadhukhan > <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/~kaddepalli/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/~kaddepalli/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/~kaddepalli/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. >
