RFR: 8170552: [macosx] Wrong rendering of diacritics on macOS

classic Classic list List threaded Threaded
7 messages Options
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

RFR: 8170552: [macosx] Wrong rendering of diacritics on macOS

Dmitry Batrak
Hello,

I'd like to propose a fix for JDK-8170552.
  bug: https://bugs.openjdk.java.net/browse/JDK-8170552
  webrev: http://cr.openjdk.java.net/~dbatrak/8170552/webrev.00/
 
I don't have a Committer status, so I'll require a sponsor.

I've proposed this patch earlier via support ticket (it's also attached to JIRA
issue). It brings the code used to detect complex text when working with system
fonts on macOS on par with similar code used for other fonts and platforms.

As compared to the previously provided patch, I've also added a test case. It
depends on specifics of the font being used (Menlo), but I couldn't think of
another way to create an automated test for this issue.

Best regards,
Dmitry Batrak
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: RFR: 8170552: [macosx] Wrong rendering of diacritics on macOS

Philip Race
Looks good to me.
It used to be a pain keeping several places in sync, hence the single
util function.
I guess that pre-dated the Apple code and we didn't realise it had its
own copy ..
So much code ... so little time ...

-phil.

On 02/10/2017 01:07 AM, Dmitry Batrak wrote:

> Hello,
>
> I'd like to propose a fix for JDK-8170552.
>   bug: https://bugs.openjdk.java.net/browse/JDK-8170552
>   webrev: http://cr.openjdk.java.net/~dbatrak/8170552/webrev.00/ 
> <http://cr.openjdk.java.net/%7Edbatrak/8170552/webrev.00/>
>
> I don't have a Committer status, so I'll require a sponsor.
>
> I've proposed this patch earlier via support ticket (it's also
> attached to JIRA
> issue). It brings the code used to detect complex text when working
> with system
> fonts on macOS on par with similar code used for other fonts and
> platforms.
>
> As compared to the previously provided patch, I've also added a test
> case. It
> depends on specifics of the font being used (Menlo), but I couldn't
> think of
> another way to create an automated test for this issue.
>
> Best regards,
> Dmitry Batrak

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: RFR: 8170552: [macosx] Wrong rendering of diacritics on macOS

Dmitry Markov
Hi Dmitry,

The fix looks good to me too, but I am not a reviewer. So  you need one
more +1 from someone else with reviewer status.
I will sponsor integration of the fix once it is finally approved.

Thanks,
Dmitry
On 11/02/2017 02:48, Phil Race wrote:

> Looks good to me.
> It used to be a pain keeping several places in sync, hence the single
> util function.
> I guess that pre-dated the Apple code and we didn't realise it had its
> own copy ..
> So much code ... so little time ...
>
> -phil.
>
> On 02/10/2017 01:07 AM, Dmitry Batrak wrote:
>> Hello,
>>
>> I'd like to propose a fix for JDK-8170552.
>>   bug: https://bugs.openjdk.java.net/browse/JDK-8170552
>>   webrev: http://cr.openjdk.java.net/~dbatrak/8170552/webrev.00/ 
>> <http://cr.openjdk.java.net/%7Edbatrak/8170552/webrev.00/>
>>
>> I don't have a Committer status, so I'll require a sponsor.
>>
>> I've proposed this patch earlier via support ticket (it's also
>> attached to JIRA
>> issue). It brings the code used to detect complex text when working
>> with system
>> fonts on macOS on par with similar code used for other fonts and
>> platforms.
>>
>> As compared to the previously provided patch, I've also added a test
>> case. It
>> depends on specifics of the font being used (Menlo), but I couldn't
>> think of
>> another way to create an automated test for this issue.
>>
>> Best regards,
>> Dmitry Batrak
>

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: RFR: 8170552: [macosx] Wrong rendering of diacritics on macOS

Vadim Pakhnushev
+1

Vadim

On 11/02/2017 16:15, dmitry markov wrote:

> Hi Dmitry,
>
> The fix looks good to me too, but I am not a reviewer. So  you need
> one more +1 from someone else with reviewer status.
> I will sponsor integration of the fix once it is finally approved.
>
> Thanks,
> Dmitry
> On 11/02/2017 02:48, Phil Race wrote:
>> Looks good to me.
>> It used to be a pain keeping several places in sync, hence the single
>> util function.
>> I guess that pre-dated the Apple code and we didn't realise it had
>> its own copy ..
>> So much code ... so little time ...
>>
>> -phil.
>>
>> On 02/10/2017 01:07 AM, Dmitry Batrak wrote:
>>> Hello,
>>>
>>> I'd like to propose a fix for JDK-8170552.
>>>   bug: https://bugs.openjdk.java.net/browse/JDK-8170552
>>>   webrev: http://cr.openjdk.java.net/~dbatrak/8170552/webrev.00/ 
>>> <http://cr.openjdk.java.net/%7Edbatrak/8170552/webrev.00/>
>>>
>>> I don't have a Committer status, so I'll require a sponsor.
>>>
>>> I've proposed this patch earlier via support ticket (it's also
>>> attached to JIRA
>>> issue). It brings the code used to detect complex text when working
>>> with system
>>> fonts on macOS on par with similar code used for other fonts and
>>> platforms.
>>>
>>> As compared to the previously provided patch, I've also added a test
>>> case. It
>>> depends on specifics of the font being used (Menlo), but I couldn't
>>> think of
>>> another way to create an automated test for this issue.
>>>
>>> Best regards,
>>> Dmitry Batrak
>>
>

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: RFR: 8170552: [macosx] Wrong rendering of diacritics on macOS

Sergey Bylokhov
I am not sure, but Is the apache license in the test can be accepted in the openjdk? I had an impression that all code should by under GPL v2 or  GPL v2 + the Classpath exception.

>
>
> +1
>
> Vadim
>
> On 11/02/2017 16:15, dmitry markov wrote:
>> Hi Dmitry,
>>
>> The fix looks good to me too, but I am not a reviewer. So  you need one more +1 from someone else with reviewer status.
>> I will sponsor integration of the fix once it is finally approved.
>>
>> Thanks,
>> Dmitry
>> On 11/02/2017 02:48, Phil Race wrote:
>>> Looks good to me.
>>> It used to be a pain keeping several places in sync, hence the single util function.
>>> I guess that pre-dated the Apple code and we didn't realise it had its own copy ..
>>> So much code ... so little time ...
>>>
>>> -phil.
>>>
>>> On 02/10/2017 01:07 AM, Dmitry Batrak wrote:
>>>> Hello,
>>>>
>>>> I'd like to propose a fix for JDK-8170552.
>>>>  bug: https://bugs.openjdk.java.net/browse/JDK-8170552
>>>>  webrev: http://cr.openjdk.java.net/~dbatrak/8170552/webrev.00/ <http://cr.openjdk.java.net/%7Edbatrak/8170552/webrev.00/>
>>>>
>>>> I don't have a Committer status, so I'll require a sponsor.
>>>>
>>>> I've proposed this patch earlier via support ticket (it's also attached to JIRA
>>>> issue). It brings the code used to detect complex text when working with system
>>>> fonts on macOS on par with similar code used for other fonts and platforms.
>>>>
>>>> As compared to the previously provided patch, I've also added a test case. It
>>>> depends on specifics of the font being used (Menlo), but I couldn't think of
>>>> another way to create an automated test for this issue.
>>>>
>>>> Best regards,
>>>> Dmitry Batrak
>>>
>>
>

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: RFR: 8170552: [macosx] Wrong rendering of diacritics on macOS

Philip Race
You are right !
I missed that. OpenJDK rules rquire it be contributed under GPL v2 (not
classpath)
The test needs to be updated or deleted ASAP (today would be good).

-phil.

On 2/15/17, 4:47 AM, Sergey Bylokhov wrote:

> I am not sure, but Is the apache license in the test can be accepted in the openjdk? I had an impression that all code should by under GPL v2 or  GPL v2 + the Classpath exception.
>
>>
>> +1
>>
>> Vadim
>>
>> On 11/02/2017 16:15, dmitry markov wrote:
>>> Hi Dmitry,
>>>
>>> The fix looks good to me too, but I am not a reviewer. So  you need one more +1 from someone else with reviewer status.
>>> I will sponsor integration of the fix once it is finally approved.
>>>
>>> Thanks,
>>> Dmitry
>>> On 11/02/2017 02:48, Phil Race wrote:
>>>> Looks good to me.
>>>> It used to be a pain keeping several places in sync, hence the single util function.
>>>> I guess that pre-dated the Apple code and we didn't realise it had its own copy ..
>>>> So much code ... so little time ...
>>>>
>>>> -phil.
>>>>
>>>> On 02/10/2017 01:07 AM, Dmitry Batrak wrote:
>>>>> Hello,
>>>>>
>>>>> I'd like to propose a fix for JDK-8170552.
>>>>>   bug: https://bugs.openjdk.java.net/browse/JDK-8170552
>>>>>   webrev: http://cr.openjdk.java.net/~dbatrak/8170552/webrev.00/<http://cr.openjdk.java.net/%7Edbatrak/8170552/webrev.00/>
>>>>>
>>>>> I don't have a Committer status, so I'll require a sponsor.
>>>>>
>>>>> I've proposed this patch earlier via support ticket (it's also attached to JIRA
>>>>> issue). It brings the code used to detect complex text when working with system
>>>>> fonts on macOS on par with similar code used for other fonts and platforms.
>>>>>
>>>>> As compared to the previously provided patch, I've also added a test case. It
>>>>> depends on specifics of the font being used (Menlo), but I couldn't think of
>>>>> another way to create an automated test for this issue.
>>>>>
>>>>> Best regards,
>>>>> Dmitry Batrak
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: RFR: 8170552: [macosx] Wrong rendering of diacritics on macOS

Dmitry Markov
I have opened JDK-8175025 for that. Will send review soon.

Thanks,
Dmitry
On 15/02/2017 18:36, Philip Race wrote:
You are right !
I missed that. OpenJDK rules rquire it be contributed under GPL v2 (not classpath)
The test needs to be updated or deleted ASAP (today would be good).

-phil.

On 2/15/17, 4:47 AM, Sergey Bylokhov wrote:
I am not sure, but Is the apache license in the test can be accepted in the openjdk? I had an impression that all code should by under GPL v2 or  GPL v2 + the Classpath exception.


+1

Vadim

On 11/02/2017 16:15, dmitry markov wrote:
Hi Dmitry,

The fix looks good to me too, but I am not a reviewer. So  you need one more +1 from someone else with reviewer status.
I will sponsor integration of the fix once it is finally approved.

Thanks,
Dmitry
On 11/02/2017 02:48, Phil Race wrote:
Looks good to me.
It used to be a pain keeping several places in sync, hence the single util function.
I guess that pre-dated the Apple code and we didn't realise it had its own copy ..
So much code ... so little time ...

-phil.

On 02/10/2017 01:07 AM, Dmitry Batrak wrote:
Hello,

I'd like to propose a fix for JDK-8170552.
  bug: https://bugs.openjdk.java.net/browse/JDK-8170552
  webrev: http://cr.openjdk.java.net/~dbatrak/8170552/webrev.00/<http://cr.openjdk.java.net/%7Edbatrak/8170552/webrev.00/>

I don't have a Committer status, so I'll require a sponsor.

I've proposed this patch earlier via support ticket (it's also attached to JIRA
issue). It brings the code used to detect complex text when working with system
fonts on macOS on par with similar code used for other fonts and platforms.

As compared to the previously provided patch, I've also added a test case. It
depends on specifics of the font being used (Menlo), but I couldn't think of
another way to create an automated test for this issue.

Best regards,
Dmitry Batrak

Loading...