[prev in list] [next in list] [prev in thread] [next in thread]
List: openjdk-2d-dev
Subject: Re: [OpenJDK 2D-Dev] [PATCH] 8218914: Support fonts installed per-user on Windows 10
From: Phil Race <philip.race () oracle ! com>
Date: 2019-02-27 21:35:36
Message-ID: 4e238824-5bd0-1734-9a64-20e8c2d206a7 () oracle ! com
[Download RAW message or body]
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 <philip.race@oracle.com
> > <mailto:philip.race@oracle.com>> 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
> > > "The Drive To Develop"
> > >
> >
> --
> Mikhail Filippov
> Software Developer
> JetBrains
> http://jetbrains.com
> "The Drive To Develop"
[Attachment #3 (text/html)]
<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
</head>
<body text="#000000" bgcolor="#FFFFFF">
I have made sure this builds OK and that it passes at least basic
tests.<br>
Line 514 is MUCH more than 80 chars. Please break it.<br>
<br>
Once you get a 2nd review, your sponsor can push it to jdk/client.<br>
<br>
-phil<br>
<br>
<div class="moz-cite-prefix">On 2/19/19 5:57 AM, Mikhail Filippov
wrote:<br>
</div>
<blockquote type="cite"
cite="mid:C4A8FCEC-DFC7-48AD-964F-E9B0692F9D9D@jetbrains.com">
<meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
New webrev with fixes: <a
href="http://cr.openjdk.java.net/~dbatrak/8218914/webrev.02/"
class="" moz-do-not-send="true">http://cr.openjdk.java.net/~dbatrak/8218914/webrev.02/</a><br
class="">
<div class="">
<div style="color: rgb(0, 0, 0); letter-spacing: normal;
text-align: start; text-indent: 0px; text-transform: none;
white-space: normal; word-spacing: 0px;
-webkit-text-stroke-width: 0px; word-wrap: break-word;
-webkit-nbsp-mode: space; line-break: after-white-space;"
class="">
<div style="color: rgb(0, 0, 0); letter-spacing: normal;
text-align: start; text-indent: 0px; text-transform: none;
white-space: normal; word-spacing: 0px;
-webkit-text-stroke-width: 0px; word-wrap: break-word;
-webkit-nbsp-mode: space; line-break: after-white-space;"
class=""><br class="">
</div>
</div>
</div>
<div><br class="">
<blockquote type="cite" class="">
<div class="">On 15 Feb 2019, at 19:31, Phil Race <<a
href="mailto:philip.race@oracle.com" class=""
moz-do-not-send="true">philip.race@oracle.com</a>>
wrote:</div>
<br class="Apple-interchange-newline">
<div class="">
<meta http-equiv="Content-Type" content="text/html;
charset=UTF-8" class="">
<div text="#000000" bgcolor="#FFFFFF" class=""> >
8218914: Handle the case when fonts are installed into
user registry key. This is the default behaviour since
Windows 10 1809.<br class="">
<br class="">
When you get to the point of preparing a changeset, this
line should have the bug synopsis.<br class="">
The text you have here is better placed on the "Summary:"
line.<br class="">
</div>
</div>
</blockquote>
<blockquote type="cite" class="">
<div class="">
<div text="#000000" bgcolor="#FFFFFF" class="">You seem to
have lines > 80 chars. Please fix.</div>
</div>
</blockquote>
I attached new webrev without commit message.</div>
<div><br class="">
<blockquote type="cite" class="">
<div class="">
<div text="#000000" bgcolor="#FFFFFF" class=""> What does
Windows do if a user installs a different version of a
font already installed on the system ?<br class="">
- Refuse to install it ?<br class="">
- Use the system one ?<br class="">
- Use the user one ?<br class="">
<br class="">
If it refuses to install it, we can ignore that problem.
If it prefers one, we should make sure<br class="">
we do the same.</div>
</div>
</blockquote>
</div>
<div>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.</div>
<div><br class="">
</div>
<div><br class="">
<blockquote type="cite" class="">
<div class="">
<div text="#000000" bgcolor="#FFFFFF" class=""> I think the
comment <br class="">
<br class="">
<pre class=""><span class="changed">/* Starting from Windows 10 Preview \
Build 17704 fonts are installed into user's home folder by default,</span></pre> <br \
class=""> can be misconstrued. It could be read as ALL fonts are
installed into a user folder and<br class="">
there is no more system location. I think you actually
mean<br class="">
<br class="">
<pre class=""><span class="changed">/* 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.
*/ </span></pre>
</div>
</div>
</blockquote>
Comment fixed.</div>
<div><br class="">
<blockquote type="cite" class="">
<div class="">
<div text="#000000" bgcolor="#FFFFFF" class=""> Have you
tested this on a machine with at least several user fonts
installed and<br class="">
verified we still get ALL the same system fonts as well as
the new user fonts ?<br class="">
<br class="">
Have you verified what this does on older OS versions ?<br
class="">
</div>
</div>
</blockquote>
On previous Windows version user fonts key not exists and fonts
loading only from the system-wide key. </div>
<div><br class="">
</div>
<div><br class="">
<blockquote type="cite" class="">
<div class="">
<div text="#000000" bgcolor="#FFFFFF" class=""> -phil.<br
class="">
<br class="">
<div class="moz-cite-prefix">On 2/15/19 6:23 AM, Mikhail
Filippov wrote:<br class="">
</div>
<blockquote type="cite"
cite="mid:0C833624-5874-4B2F-A134-EBE98902EEE0@jetbrains.com"
class="">
<meta http-equiv="content-type" content="text/html;
charset=UTF-8" class="">
<div class="">Hi. Please review the fix. </div>
<div class=""><br class="">
</div>
<div class="">patch: attached to message.</div>
<div class="">bug: <a
href="https://bugs.openjdk.java.net/browse/JDK-8218914"
class="" \
moz-do-not-send="true">https://bugs.openjdk.java.net/browse/JDK-8218914</a></div> \
<div class="">webrev: <a
href="http://cr.openjdk.java.net/~dbatrak/8218914/webrev.01/"
class="" \
moz-do-not-send="true">http://cr.openjdk.java.net/~dbatrak/8218914/webrev.01/</a></div>
<div class=""><br class="">
</div>
<div class="">Description:</div>
<div class="">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: "<a
href="https://blogs.windows.com/windowsexperience/2018/06/27/announcing-windows-10-insider-preview-build-17704/"
class="" \
moz-do-not-send="true">https://blogs.windows.com/windowsexperience/2018/06/27/announcing-windows-10-insider-preview-build-17704/</a>"
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.</div>
<br class="">
<br class="">
<fieldset class="mimeAttachmentHeader"></fieldset>
<meta http-equiv="content-type" content="text/html;
charset=UTF-8" class="">
<br class="">
<div class="">
<div style="letter-spacing: normal; text-align: start;
text-indent: 0px; text-transform: none; white-space:
normal; word-spacing: 0px;
-webkit-text-stroke-width: 0px; word-wrap:
break-word; -webkit-nbsp-mode: space; line-break:
after-white-space;" class="">
<div style="letter-spacing: normal; text-align:
start; text-indent: 0px; text-transform: none;
white-space: normal; word-spacing: 0px;
-webkit-text-stroke-width: 0px; word-wrap:
break-word; -webkit-nbsp-mode: space; line-break:
after-white-space;" class="">
<div class="">--</div>
<div class="">Mikhail Filippov<span class="Apple-tab-span" \
style="white-space: pre;"> </span></div> <div class="">Software Developer</div>
<div class="">JetBrains</div>
<div class=""><a href="http://jetbrains.com"
class="" \
moz-do-not-send="true">http://jetbrains.com</a></div> <div class="">"The Drive To \
Develop"</div> </div>
</div>
</div>
<br class="">
</blockquote>
<br class="">
</div>
</div>
</blockquote>
</div>
<div class="">--</div>
<div class="">Mikhail Filippov<span class="Apple-tab-span" style="white-space: \
pre;"> </span></div> <div class="">Software Developer</div>
<div class="">JetBrains</div>
<div class=""><a href="http://jetbrains.com" class=""
moz-do-not-send="true">http://jetbrains.com</a></div>
<div class="">"The Drive To Develop"</div>
</blockquote>
<br>
</body>
</html>
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic