[prev in list] [next in list] [prev in thread] [next in thread]
List: kde-panel-devel
Subject: D9070: KDE platform plugin: don't force default stylename on user-specified fonts
From: David Faure <noreply () phabricator ! kde ! org>
Date: 2018-01-28 22:25:46
Message-ID: 20180128222546.1.853AA780EAEA0DE1 () phabricator ! kde ! org
[Download RAW message or body]
dfaure added a comment.
In https://phabricator.kde.org/D9070#197332, @rjvbb wrote:
> What happens when you remove the `,Regular`
That fixes the problem indeed.
>> Workaround: enabling the line that calls \
QApplication::setDesktopSettingsAware(false). >> Proof that this code is guilty: \
commenting out the fromString line makes it work, too. >
> Which line is that, where?
In the code shown by this patch ;) Line 80.
But the test you suggested that I do above is further proof anyhow.
>> What really confuses me is: why isn't it all set up so that this \
platform-plugin code provides *default* fonts, and so that what apps do overrides \
that? >
> That is what used to be the case, and that's why I think the commit that added \
the setStyleName call to kfontsettingsdata.cpp ought to be reverted. (I haven't gone \
back to re-read the commit message, but IIRC it was mostly made to align with the way \
fonts are saved, without questioning whether they should always be saved like that.) \
But even without setStyleName call (i.e. with your patch) I get the bug, due to \
call to QFont::fromString.
REPOSITORY
R135 Integration for Qt applications in Plasma
REVISION DETAIL
https://phabricator.kde.org/D9070
To: rjvbb, #frameworks, davidedmundson, graesslin, cfeck, dfaure
Cc: rdieter, abetts, anthonyfieroni, ngraham, cfeck, fvogt, plasma-devel, ZrenBot, \
progwolff, lesliezhai, ali-mohamed, jensreuterberg, sebas, apol, mart
[Attachment #3 (unknown)]
<table><tr><td style="">dfaure added a comment.
</td><a style="text-decoration: none; padding: 4px 8px; margin: 0 8px 8px; float: \
right; color: #464C5C; font-weight: bold; border-radius: 3px; background-color: \
#F7F7F9; background-image: linear-gradient(to bottom,#fff,#f1f0f1); display: \
inline-block; border: 1px solid rgba(71,87,120,.2);" \
href="https://phabricator.kde.org/D9070" rel="noreferrer">View \
Revision</a></tr></table><br /><div><div><blockquote style="border-left: 3px solid \
#8C98B8; color: #6B748C;
font-style: italic;
margin: 4px 0 12px 0;
padding: 8px 12px;
background-color: #F8F9FC;">
<div style="font-style: normal;
padding-bottom: 4px;">In <a href="https://phabricator.kde.org/D9070#197332" \
style="background-color: #e7e7e7; border-color: #e7e7e7;
border-radius: 3px;
padding: 0 4px;
font-weight: bold;
color: black;text-decoration: none;" rel="noreferrer">D9070#197332</a>, <a \
href="https://phabricator.kde.org/p/rjvbb/" style=" border-color: #f1f7ff;
color: #19558d;
background-color: #f1f7ff;
border: 1px solid transparent;
border-radius: 3px;
font-weight: bold;
padding: 0 4px;" rel="noreferrer">@rjvbb</a> wrote:</div>
<div style="margin: 0;
padding: 0;
border: 0;
color: rgb(107, 116, 140);"><p>What happens when you remove the <tt \
style="background: #ebebeb; font-size: 13px;">,Regular</tt></p></div> </blockquote>
<p>That fixes the problem indeed.</p>
<blockquote style="border-left: 3px solid #a7b5bf; color: #464c5c; font-style: \
italic; margin: 4px 0 12px 0; padding: 4px 12px; background-color: \
#f8f9fc;"><blockquote style="border-left: 3px solid #a7b5bf; color: #464c5c; \
font-style: italic; margin: 4px 0 12px 0; padding: 4px 12px; background-color: \
#f8f9fc;"><div class="remarkup-code-block" style="margin: 12px 0;" \
data-code-lang="text" data-sigil="remarkup-code-block"><pre class="remarkup-code" \
style="font: 11px/15px "Menlo", "Consolas", "Monaco", \
monospace; padding: 12px; margin: 0; background: rgba(71, 87, 120, \
0.08);">Workaround: enabling the line that calls \
QApplication::setDesktopSettingsAware(false). Proof that this code is guilty: \
commenting out the fromString line makes it work, too.</pre></div></blockquote>
<p>Which line is that, where?</p></blockquote>
<p>In the code shown by this patch ;) Line 80.<br />
But the test you suggested that I do above is further proof anyhow.</p>
<blockquote style="border-left: 3px solid #a7b5bf; color: #464c5c; font-style: \
italic; margin: 4px 0 12px 0; padding: 4px 12px; background-color: \
#f8f9fc;"><blockquote style="border-left: 3px solid #a7b5bf; color: #464c5c; \
font-style: italic; margin: 4px 0 12px 0; padding: 4px 12px; background-color: \
#f8f9fc;"><div class="remarkup-code-block" style="margin: 12px 0;" \
data-code-lang="text" data-sigil="remarkup-code-block"><pre class="remarkup-code" \
style="font: 11px/15px "Menlo", "Consolas", "Monaco", \
monospace; padding: 12px; margin: 0; background: rgba(71, 87, 120, 0.08);">What \
really confuses me is: why isn't it all set up so that this platform-plugin code \
provides *default* fonts, and so that what apps do overrides \
that?</pre></div></blockquote>
<p>That is what used to be the case, and that's why I think the commit that \
added the setStyleName call to kfontsettingsdata.cpp ought to be reverted. (I \
haven't gone back to re-read the commit message, but IIRC it was mostly made to \
align with the way fonts are saved, without questioning whether they should always be \
saved like that.)</p></blockquote>
<p>But even without setStyleName call (i.e. with your patch) I get the bug, due to \
call to QFont::fromString.</p></div></div><br \
/><div><strong>REPOSITORY</strong><div><div>R135 Integration for Qt applications in \
Plasma</div></div></div><br /><div><strong>REVISION DETAIL</strong><div><a \
href="https://phabricator.kde.org/D9070" \
rel="noreferrer">https://phabricator.kde.org/D9070</a></div></div><br \
/><div><strong>To: </strong>rjvbb, Frameworks, davidedmundson, graesslin, cfeck, \
dfaure<br /><strong>Cc: </strong>rdieter, abetts, anthonyfieroni, ngraham, cfeck, \
fvogt, plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, \
sebas, apol, mart<br /></div>
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic