<Swing Dev> [13] RFR: JDK-8218674 - HTML Tooltip with "img=src" on component doesn't show

[Krishna Addepalli]

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.sadhukhan at 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.sadhukhan at oracle.com <mailto:prasanta.sadhukhan at 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.Bylokhov at oracle.com> <mailto:Sergey.Bylokhov at 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.addepalli at oracle.com> <mailto:krishna.addepalli at oracle.com>; swing-dev at openjdk.java.net <mailto:swing-dev at 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.
>>> 
>> 
> 

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.java.net/pipermail/swing-dev/attachments/20190304/0378cd49/attachment.html>