[Okular-devel] Review Request 111681: TextDocumentGenerator: Use black as default text color

[Christoph Feck]

> On Sept. 25, 2013, 8:40 p.m., Albert Astals Cid wrote:
> > I'm again confused, isn't the idea that we respect the colors specified on the file? Or not? Jaydeep, what's the final decision we took?
> 
> Christoph Feck wrote:
>     Albert, the issue is that some documents do not indicate text color, they just assume.
> 
> Albert Astals Cid wrote:
>     Right, so if they assume and we follow the color scheme, all is nice, no? You'll get your weird black on red as defined on your color scheme? Or that's something we don't like and we want it to be black on white not obeying your color scheme?

Yes, you can follow the color scheme, but then you have to do it for both background and foreground. But right now, Okular uses a white background.

In the description I explained why I think following the color scheme might be bad: If you follow the color scheme, then you can break (i.e. render invisible) documents that _do_ set text colors, especially for users that use a dark color scheme.

Of course, a third option would be to ignore all colors in the document, and force both background and foreground to use the color scheme, but then "colorful" documents would look boring.


- Christoph


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/111681/#review40816
-----------------------------------------------------------


On Aug. 20, 2013, 10:40 a.m., Christoph Feck wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/111681/
> -----------------------------------------------------------
> 
> (Updated Aug. 20, 2013, 10:40 a.m.)
> 
> 
> Review request for Okular.
> 
> 
> Description
> -------
> 
> As indicated in bug 322547, some documents do not specify a text color, and probably assume the default text color to be black. QTextDocument, however, defaults to using the system text color.
> 
> This patch changes the default text color to Qt::black. It should affect epub, fb2, odt, and plain text generators.
> 
> I think it is better to use this approach instead of changing the paper color to use the system background color (see bug 253583), because
> 
> 1) the document might specify a text color in some places,
> 
> 2) the user is able to change the fg/bg colors anyway using Okular's Accessibility options, and those probably expect black on white.
> 
> 
> This addresses bugs 253583 and 322547.
>     http://bugs.kde.org/show_bug.cgi?id=253583
>     http://bugs.kde.org/show_bug.cgi?id=322547
> 
> 
> Diffs
> -----
> 
>   core/textdocumentgenerator.cpp b260b3f 
> 
> Diff: http://git.reviewboard.kde.org/r/111681/diff/
> 
> 
> Testing
> -------
> 
> I tested the document from bug 322547 comment #3.
> 
> 
> Thanks,
> 
> Christoph Feck
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/okular-devel/attachments/20130925/b22c4249/attachment-0001.html>