<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="8" style="border: 1px #c9c399 solid;">
<tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="https://git.reviewboard.kde.org/r/116633/">https://git.reviewboard.kde.org/r/116633/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On March 6th, 2014, 1:55 p.m. UTC, <b>Mark Gaiser</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">How will this work with other non KDE apps (like Chrome), will they simply pickup the Oxygen font?
I'm asking because fonts are working just fine now. If i open a GTK app in KDE it shows the fonts in the same manner as a KDE app would show them. That is consistent and quite likable :)</pre>
</blockquote>
<p>On March 6th, 2014, 2:16 p.m. UTC, <b>Sebastian Kügler</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">They won't pick up the font. Nothing we can do about that in Plasma. Needs the distro or the Chrome developers to do it. The Oxygen Widget style for GTK could probably pick it up, though.
This is unrelated to this patch, however.</pre>
</blockquote>
<p>On March 6th, 2014, 2:33 p.m. UTC, <b>Mark Gaiser</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Well, if this patch gives the user a broken consistency (Chrome is being used by lots of people, including KDE folks) then i think it might be best to keep the defaults consistent and keep it at sans-serif.
That would at least keep the default consistent.
Another approach might be (don't know if that's done already) to generate a local user font.conf which changes the default sans-serif font to whatever font is set in KDE's font settings.
Note: Chrome is just a sane example here. Others would be gparted, firefox, <fill in any gtk app></pre>
</blockquote>
<p>On March 6th, 2014, 3:08 p.m. UTC, <b>Martin Gräßlin</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">@Mark: well it's orthogonal to this change. We need to start somewhere the transition. If we block the first patch because it introduces inconsistancies, we will never be able to adress it. Thus I think it's the right approach to start with the what we have control over.</pre>
</blockquote>
<p>On March 6th, 2014, 3:10 p.m. UTC, <b>Aleix Pol Gonzalez</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">@Mark: Nobody assures Chrome will be using "Sans Serif" either, this patch doesn't make a difference in this regard...</pre>
</blockquote>
<p>On March 6th, 2014, 3:45 p.m. UTC, <b>Mark Gaiser</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">@Martin, @Aleix; To be clear, I'm not against this change at all. I'm only against the result it will have. Sure, you need to start somewhere. In this case that would be assuring that all applications follow the font settings that are in kde's font settings. How one should tackle that, i don't know. I guess it's possible with a local fonts.conf file.
@Aleix not a strong argument. GUI interfaces almost always use the default font settings. My exact point is that those defaults _won't_ change for GTK apps with this patch.</pre>
</blockquote>
<p>On March 6th, 2014, 5:33 p.m. UTC, <b>Àlex Fiestas</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Well supporting gtk app's is as simple as writing the configuration file like or kcm-gtk-style does. If chromium is not reading that and rather using their own config file (which I doubt) then it would be their bug, not ours.
Also, imho we should ask KDE/PLasma distributions to set this settings by default distro-wise.</pre>
</blockquote>
<p>On March 6th, 2014, 5:35 p.m. UTC, <b>Àlex Fiestas</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Just to make my point clear, I'm not against this review but I think it should be followed by another one configuring other toolkits fonts :)</pre>
</blockquote>
<p>On March 6th, 2014, 10:44 p.m. UTC, <b>Sebastian Kügler</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Yes, it makes sense to enforce the font setting also in other places. Our GTK KCM hasn't been ported yet, it seems, but once it is ported, we should definitely make sure it uses the Oxygen font as well (if the Oxygen GTK style doesn't already do so).
You're listed as maintainer for the frameworksintegration repo, so if you think the code in this review is OK, please hit ship it. :)</pre>
</blockquote>
</blockquote>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">The GTK kcm will suggest as a default whatever the QPA suggests, but then you have to go in the kcm at least once.</pre>
<br />
<p>- Aleix</p>
<br />
<p>On March 6th, 2014, 4:36 p.m. UTC, Sebastian Kügler wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('https://git.reviewboard.kde.org/static/rb/images/review_request_box_top_bg.ab6f3b1072c9.png'); background-position: left top; background-repeat: repeat-x; border: 1px black solid;">
<tr>
<td>
<div>Review request for Plasma and Àlex Fiestas.</div>
<div>By Sebastian Kügler.</div>
<p style="color: grey;"><i>Updated March 6, 2014, 4:36 p.m.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
frameworkintegration
</div>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Description </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Change default font settings to Oxygen font
Depend on kde:oxygen-fonts being installed. oxygen-fonts installs a file called OxygenFontConfig.cmake, which is used for checking the dependency is available.</pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Testing </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Started kate without a kdeglobals file being present, fonts are picked up correctly.</pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>CMakeLists.txt <span style="color: grey">(e249554)</span></li>
<li>src/platformtheme/kfontsettingsdata.cpp <span style="color: grey">(62990ce)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/116633/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>