<table><tr><td style="">albertfreeman edited the summary of this revision. <a href="https://phabricator.kde.org/transactions/detail/PHID-XACT-DREV-tgltqbe22r67py6/" rel="noreferrer">(Show Details)</a>
</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/D8051" rel="noreferrer">View Revision</a></tr></table><br /><div><strong>CHANGES TO REVISION SUMMARY</strong><div><div style="white-space: pre-wrap; color: #74777D;"><div style="padding: 8px 0;">...</div>Reasons for this change:<span style="padding: 0 2px; color: #333333; background: rgba(151, 234, 151, .6);"><br />
<br />
<br />
> Youtube video with ~100 upvotes and ~60'000 views (although it changes the pages color instead of the background color)(https://www.youtube.com/watch?v=_N7PgpkH8lM)<br />
> <br />
> So the code is useful for accessibility, eye strain and aesthetic reasons.</span><br />
<span style="padding: 0 2px; color: #333333; background: rgba(251, 175, 175, .7);"> Youtube video with ~100 upvotes and ~60'000 views (although it changes the pages color instead of the background color)(https://www.youtube.com/watch?v=_N7PgpkH8lM)<br />
<br />
So the code is useful for accessibility, eye strain and aesthetic reasons.</span><span style="padding: 0 2px; color: #333333; background: rgba(151, 234, 151, .6);">> <br />
> Additionally, power consumption is minorly affected by color and depends on display technology, sometimes black uses more power, othertimes white does<br />
> (https://www.quora.com/Does-a-white-background-use-more-energy-on-an-LCD-than-if-it-was-set-to-black)</span><br />
<br />
<span style="padding: 0 2px; color: #333333; background: rgba(251, 175, 175, .7);"> Additionally</span><span style="padding: 0 2px; color: #333333; background: rgba(151, 234, 151, .6);">Reasons for this change given by Henrik Fehlauer (rkflx at lab12.net):<br />
> Thanks for providing a patch instead of just complaining on Bugzilla. As Reviewboard will become readonly in the near future, I would encourage you to bring this review over to Phabricator</span>, <span style="padding: 0 2px; color: #333333; background: rgba(251, 175, 175, .7);">power consumption is minorly affected by color and depends on display technology</span><span style="padding: 0 2px; color: #333333; background: rgba(151, 234, 151, .6);">so we can discuss further improvements (if you are still interested</span>, <span style="padding: 0 2px; color: #333333; background: rgba(251, 175, 175, .7);">sometimes black uses more power</span><span style="padding: 0 2px; color: #333333; background: rgba(151, 234, 151, .6);">which I hope</span>, <span style="padding: 0 2px; color: #333333; background: rgba(251, 175, 175, .7);">othertimes white does</span><span style="padding: 0 2px; color: #333333; background: rgba(151, 234, 151, .6);">as the latest version of your patch is much improved).</span><br />
<span style="padding: 0 2px; color: #333333; background: rgba(251, 175, 175, .7);"> (https://www.quora.com/Does-a-white-background-use-more-energy-on-an-LCD-than-if-it-was-set-to-black)<br />
<br />
Reasons for this change by Henrik Fehlauer (rkflx at lab12.net):</span><span style="padding: 0 2px; color: #333333; background: rgba(151, 234, 151, .6);">> <br />
> For example, you could improve the wording and layout:<br />
> [ ] Use custom background color: [color chooser] (← Note inversion of checkbox logic, and disable colour chooser if checkbox is unchecked. Possibly decrease the colour chooser's width.)</span><br />
<span style="padding: 0 2px; color: #333333; background: rgba(251, 175, 175, .7);">Thanks for providing a patch instead of just complaining on Bugzilla. As Reviewboard will become readonly in the near future, I would encourage you to bring this review over to Phabricator, so we can discuss further improvements (if you are still interested, which I hope, as the latest version of your patch is much improved).<br />
<br />
For example</span><span style="padding: 0 2px; color: #333333; background: rgba(151, 234, 151, .6);">> <br />
> Advantages: Does away with the unmeaningful "Use UI theme"</span>, <span style="padding: 0 2px; color: #333333; background: rgba(251, 175, 175, .7);">you could improve the wording</span><span style="padding: 0 2px; color: #333333; background: rgba(151, 234, 151, .6);">does not duplicate the words "background color" and is more compact (only one line)</span> and l<span style="padding: 0 2px; color: #333333; background: rgba(251, 175, 175, .7);">ayout:</span><span style="padding: 0 2px; color: #333333; background: rgba(151, 234, 151, .6);">ess cluttered.</span><br />
<span style="padding: 0 2px; color: #333333; background: rgba(251, 175, 175, .7);">[ ] Use custom background color: [color chooser] (← Note inversion of checkbox logic, and disable colour chooser if checkbox is unchecked.</span><span style="padding: 0 2px; color: #333333; background: rgba(151, 234, 151, .6);">> <br />
> @aacid:<br />
> FWIW,</span> <span style="padding: 0 2px; color: #333333; background: rgba(251, 175, 175, .7);">Possibly decrease the colour chooser's width.)<br />
<br />
Advantages: Does away with the unmeaningful "Use UI theme",</span><span style="padding: 0 2px; color: #333333; background: rgba(151, 234, 151, .6);">I'm running a patched Okular with a different (hard coded) background colour since years, because basing the non-page background colour on the window background colour gives a very ugly result in my case (I don't think the code is to blame here, it's just impossible to find an algorithm which works for every possible colour).</span> <span style="padding: 0 2px; color: #333333; background: rgba(251, 175, 175, .7);">does</span><span style="padding: 0 2px; color: #333333; background: rgba(151, 234, 151, .6);">But I'm</span> not <span style="padding: 0 2px; color: #333333; background: rgba(251, 175, 175, .7);">duplicate</span><span style="padding: 0 2px; color: #333333; background: rgba(151, 234, 151, .6);">alone,</span> the <span style="padding: 0 2px; color: #333333; background: rgba(251, 175, 175, .7);">words "background color" and is more compact (only one line) and less cluttered.<br />
<br />
@aacid:</span><span style="padding: 0 2px; color: #333333; background: rgba(151, 234, 151, .6);">following bugs have a total of 36 votes as of today:</span><br />
<span style="padding: 0 2px; color: #333333; background: rgba(251, 175, 175, .7);">FWIW, I'm running a patched Okular with a different (hard coded) background colour since years, because basing the non-page background colour on the window background colour gives a very ugly result in my case (I don't think the code is to blame here, it's just impossible to find an algorithm which works for every possible colour). But I'm not alone, the following bugs have a total of 36 votes as of today:<br />
<br />
</span><span style="padding: 0 2px; color: #333333; background: rgba(151, 234, 151, .6);">> <br />
> https://bugs.kde.org/show_bug.cgi?id=182994<br />
> </span>https://bugs.kde.org/show_bug.cgi?<span style="padding: 0 2px; color: #333333; background: rgba(251, 175, 175, .7);">id=182994</span><span style="padding: 0 2px; color: #333333; background: rgba(151, 234, 151, .6);">id=307116</span><br />
<span style="padding: 0 2px; color: #333333; background: rgba(151, 234, 151, .6);">> </span>https://bugs.kde.org/show_bug.cgi?id=3<span style="padding: 0 2px; color: #333333; background: rgba(251, 175, 175, .7);">07116</span><span style="padding: 0 2px; color: #333333; background: rgba(151, 234, 151, .6);">19736</span><br />
<span style="padding: 0 2px; color: #333333; background: rgba(151, 234, 151, .6);">> </span>https://bugs.kde.org/show_bug.cgi?id=3<span style="padding: 0 2px; color: #333333; background: rgba(251, 175, 175, .7);">19736</span><span style="padding: 0 2px; color: #333333; background: rgba(151, 234, 151, .6);">72055</span><br />
<span style="padding: 0 2px; color: #333333; background: rgba(151, 234, 151, .6);">> </span>https://bugs.kde.org/show_bug.cgi?id=3<span style="padding: 0 2px; color: #333333; background: rgba(251, 175, 175, .7);">72055</span><span style="padding: 0 2px; color: #333333; background: rgba(151, 234, 151, .6);">69627#c15</span><br />
<span style="padding: 0 2px; color: #333333; background: rgba(251, 175, 175, .7);">https://bugs.kde.org/show_bug.cgi?id=369627#c15<br />
<br />
</span><span style="padding: 0 2px; color: #333333; background: rgba(151, 234, 151, .6);">> <br />
> </span>Looking at other apps like Gwenview or Libreoffice, those often allow to change the non-page background colour, too.<br />
<span style="padding: 0 2px; color: #333333; background: rgba(251, 175, 175, .7);"><br />
</span><span style="padding: 0 2px; color: #333333; background: rgba(151, 234, 151, .6);">> <br />
> </span>As for the maintenance argument, I don't think it holds: This is no new subsystem but a trivial feature with no complex code dependencies, and we are already showing a colour selection dialog and setting colours in other places in Okular.<br />
<span style="padding: 0 2px; color: #333333; background: rgba(251, 175, 175, .7);">End of Reasons for this change by Henrik Fehlauer<br />
</span><span style="padding: 0 2px; color: #333333; background: rgba(151, 234, 151, .6);">> <br />
<br />
<br />
Other less important information:</span><br />
https://git.reviewboard.kde.org/r/130219/<div style="padding: 8px 0;">...</div></div></div></div><br /><div><strong>REPOSITORY</strong><div><div>R223 Okular</div></div></div><br /><div><strong>REVISION DETAIL</strong><div><a href="https://phabricator.kde.org/D8051" rel="noreferrer">https://phabricator.kde.org/D8051</a></div></div><br /><div><strong>To: </strong>albertfreeman, Okular<br /><strong>Cc: </strong>aacid, ltoscano, ngraham<br /></div>