<Swing Dev> RFR: [12] JDK-8208638: Instead of circle rendered in appl window, but ellipse is produced JEditor Pane

[Krishna Addepalli]

Thanks for the comments Prasanta, here is the updated webrev: http://cr.openjdk.java.net/~kaddepalli/8208638/webrev02/ <http://cr.openjdk.java.net/~kaddepalli/8208638/webrev02/>

Krishna

> On 10-Sep-2018, at 5:20 PM, Prasanta Sadhukhan <prasanta.sadhukhan at oracle.com> wrote:
> 
> yes, right. fix looks ok to me. Please change comment style in l946 and make it similar to l956. Also, l960 change "ration" to "ratio"
> 
> Regards
> Prasanta
> On 9/7/2018 9:00 PM, Krishna Addepalli wrote:
>> Hi Prasanta,
>>  
>> I think you are confused by the same name of the variables. newHeight/newWidth are in “updateImage” and also “ImageHandler::imageUpdate”.
>> imageUpdate can be called from a background thread, with new information from the image being loaded, that contains the newHeight and newWidth. Along with this, the “flags” variable lets us know what properties of the image are updated. For example, ImageObserver.HEIHGT and ImageObserver.WIDTH flags being set indicates, that this update call has fetched the image properties from the file.
>> Based on this, I cache these values, and then update them once the image load is complete.
>>  
>> Thanks,
>> Krishna
>>  
>> From: Prasanta Sadhukhan 
>> Sent: Friday, September 7, 2018 7:04 PM
>> 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> RFR: [12] JDK-8208638: Instead of circle rendered in appl window, but ellipse is produced JEditor Pane
>>  
>> Hi Krishna,
>> 
>> We obtain newHeight and specifiedHeight by same method
>> 
>> newHeight = getIntAttr(HTML.Attribute.HEIGHT, -1);
>> and 
>> 
>> specifiedHeight = getIntAttr(HTML.Attribute.HEIGHT, -1);
>> so I assume both will return the same value. So,
>> 
>> 968                         if (specifiedHeight <= 0) {
>>  969                             proportion = specifiedWidth / ((double)newWidth);
>>  970                             newHeight = (int)(proportion * newHeight);
>>  971                         }
>> if specifiedHeight is <=0 then newHeight * proportion will also be -ve or 0, isn't it? Will it not cause problem? Am I missing something?
>> 
>> Regards
>> Prasanta
>> On 9/5/2018 4:25 PM, Krishna Addepalli wrote:
>> Hi All,
>>  
>> The testfix turned out to be simpler than I thought. I just had to query the preferred size before making the frame visible, so that the image is rendered on screen properly, and now Robot is able to pick the color.
>> Here is the modified webrev: http://cr.openjdk.java.net/~kaddepalli/8208638/webrev01/ <http://cr.openjdk.java.net/%7Ekaddepalli/8208638/webrev01/>
>>  
>> Thanks,
>> Krishna
>>  
>> From: Krishna Addepalli 
>> Sent: Wednesday, September 5, 2018 9:41 AM
>> To: swing-dev at openjdk.java.net <mailto:swing-dev at openjdk.java.net>
>> Subject: Re: <Swing Dev> RFR: [12] JDK-8208638: Instead of circle rendered in appl window, but ellipse is produced JEditor Pane
>>  
>> Hi All,
>>  
>> I found an issue with the test, and am working on it to make it pass reliably. I was setting the size of the frame and then querying the preferred size, which makes it a non-test. 
>> I’m working to address it, meanwhile you could still review the fix itself.
>>  
>> Thanks,
>> Krishna
>>  
>> From: Krishna Addepalli 
>> Sent: Wednesday, September 5, 2018 12:10 AM
>> To: swing-dev at openjdk.java.net <mailto:swing-dev at openjdk.java.net>
>> Subject: RFR: [12] JDK-8208638: Instead of circle rendered in appl window, but ellipse is produced JEditor Pane
>>  
>> Hi All, 
>> Please review a fix for JDK-8208638: https://bugs.openjdk.java.net/browse/JDK-8208638 <https://bugs.openjdk.java.net/browse/JDK-8208638>
>> Webrev: http://cr.openjdk.java.net/~kaddepalli/8208638/webrev00/ <http://cr.openjdk.java.net/%7Ekaddepalli/8208638/webrev00/>
>>  
>> JEditorPane supports loading HTML content, including images, and this bug is about image scaling when either of the width/height attributes are not specified in the HTML text.
>> The problem is that scaling of the images in all cases was not handled properly. Here is the behavior that is observed in other browsers:
>> 1.       If both width and height are specified in html, then use them instead.
>> 2.       If both are not specified, then use the image dimensions.
>> 3.       If either one is specified, then proportionately adjust the other dimension to preserve the aspect ratio of the image.
>> ImageView.java implementation was not handling the last case, which is added in the fix.
>>  
>> The test is also fixed now, which was earlier relying on Robot.getPixelColor, which does not work reliably. A happy alternative was found, where in the “RootView” that is rendering the html content is retrieved from the JEditorPane,  and its preferred span is queried, which should match the case specific width/height. 
>> Tested it on all platforms (Windows, Linux(Ubuntu) and Mac) and now it reliably passes.
>>  
>> Thanks,
>> Krishna
>>  
> 
> 

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/swing-dev/attachments/20180910/a5d15fc6/attachment-0001.html>