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

[Prasanta Sadhukhan]

+1

Regards
Prasanta
On 9/10/2018 5:56 PM, Krishna Addepalli wrote:
> Thanks for the comments Prasanta, here is the updated webrev: 
> http://cr.openjdk.java.net/~kaddepalli/8208638/webrev02/ 
> <http://cr.openjdk.java.net/%7Ekaddepalli/8208638/webrev02/>
>
> Krishna
>
>> On 10-Sep-2018, at 5:20 PM, Prasanta Sadhukhan 
>> <prasanta.sadhukhan at oracle.com 
>> <mailto: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>;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
>>>     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/84e03e33/attachment-0001.html>