Looks fine.
Thank you for contribution!

On 12/03/2019 23:38, Mikhail Filippov wrote:
I looking for the second reviewer and sponsor for my font patch: 
http://cr.openjdk.java.net/~dbatrak/8218914/webrev.03/
--
Mikhail Filippov
Software Developer
JetBrains
http://jetbrains.com
“The Drive To Develop"


On 4 Mar 2019, at 21:45, Mikhail Filippov <[email protected] 
<mailto:[email protected]>> wrote:

Webrev with line-length fix: 
http://cr.openjdk.java.net/~dbatrak/8218914/webrev.03/
--
Mikhail Filippov
Software Developer
JetBrains
http://jetbrains.com <http://jetbrains.com/>
“The Drive To Develop"

On 28 Feb 2019, at 00:35, Phil Race <[email protected] 
<mailto:[email protected]>> wrote:

I have made sure this builds OK and that it passes at least basic tests.
Line 514 is MUCH more than 80 chars. Please break it.

Once you get a 2nd review, your sponsor can push it to jdk/client.

-phil

On 2/19/19 5:57 AM, Mikhail Filippov wrote:
New webrev with fixes: http://cr.openjdk.java.net/~dbatrak/8218914/webrev.02/


On 15 Feb 2019, at 19:31, Phil Race <[email protected] 
<mailto:[email protected]>> wrote:

> 8218914: Handle the case when fonts are installed into user registry key. 
This is the default behaviour since Windows 10 1809.

When you get to the point of preparing a changeset, this line should have the 
bug synopsis.
The text you have here is better placed on the "Summary:" line.
You seem to have lines > 80 chars. Please fix.
I attached new webrev without commit message.

What does Windows do if a user installs a different version of a font already 
installed on the system ?
- Refuse to install it ?
- Use the system one ?
- Use the user one ?

If it refuses to install it, we can ignore that problem. If it prefers one, we 
should make sure
we do the same.
I check this case. If you have installed system-wide and user fonts system-wide 
font preferred. I changed call order in the patch to match this behaviour.


I think the comment

/* Starting from Windows 10 Preview Build 17704 fonts are installed into user's 
home folder by default,

can be misconstrued. It could be read as ALL fonts are installed into a user 
folder and
there is no more system location. I think you actually mean

/* Starting from Windows 10 Preview Build 17704, when a user installs 
non-system fonts, * then by default they are installed in a new per-user 
location as specified in a * per user registry entry. */
Comment fixed.

Have you tested this on a machine with at least several user fonts installed and
verified we still get ALL the same system fonts as well as the new user fonts ?

Have you verified what this does on older OS versions ?
On previous Windows version user fonts key not exists and fonts loading only 
from the system-wide key.


-phil.

On 2/15/19 6:23 AM, Mikhail Filippov wrote:
Hi. Please review the fix.

patch: attached to message.
bug: https://bugs.openjdk.java.net/browse/JDK-8218914
webrev: http://cr.openjdk.java.net/~dbatrak/8218914/webrev.01/

Description:
Starting from Windows 10 Preview Build 17704 fonts are installed into the user's home 
folder by default, and are listed in user's registry section. This is Microsoft blog post 
about it: 
"https://blogs.windows.com/windowsexperience/2018/06/27/announcing-windows-10-insider-preview-build-17704/";
 I this patch I extract function for registry access and call it for two keys: 
HKEY_LOCAL_MACHINE and HKEY_CURRENT_USER. In original code fonts loading only from 
HKEY_LOCAL_MACHINE.



--
Mikhail Filippov
Software Developer
JetBrains
http://jetbrains.com <http://jetbrains.com/>
“The Drive To Develop"


--
Mikhail Filippov
Software Developer
JetBrains
http://jetbrains.com <http://jetbrains.com/>
“The Drive To Develop"





--
Best regards, Sergey.

Reply via email to