Webrev with line-length fix: http://cr.openjdk.java.net/~dbatrak/8218914/webrev.03/ -- Mikhail Filippov Software Developer JetBrains http://jetbrains.com “The Drive To Develop"
> On 28 Feb 2019, at 00:35, Phil Race <[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/ >> <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 >>>> <https://bugs.openjdk.java.net/browse/JDK-8218914> >>>> webrev: http://cr.openjdk.java.net/~dbatrak/8218914/webrev.01/ >>>> <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/ >>>> >>>> <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" >
