> On Sept. 26, 2013, 2:10 a.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?
> 
> Christoph Feck wrote:
>     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.
> 
> Albert Astals Cid wrote:
>     Why would someone set the text color and not the background color? Does 
> that happen?

Okay, here's what we need :
i)  if text color is explicitly specified in html/css use that, else use black.
ii) if background color is specified use that, else use white.
iii)link color should be independent of the color scheme ( i.e. it should be 
blue )

What your patch does,
i)  if no color is specified, it uses black for text, but if a color is 
specified it uses that color ( I checked ).
ii) Link color still depends on the color scheme. ( I saw you have included a 
line to set the link color to blue, but it follows the theme color )

It would be nice if you can do something about he background color too, because 
the implementation that we have in epubs takes care of that (see branch 
'epub-qtextdoc').


- Jaydeep


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


On Aug. 20, 2013, 4:10 p.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, 4:10 p.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
> 
>

_______________________________________________
Okular-devel mailing list
Okular-devel@kde.org
https://mail.kde.org/mailman/listinfo/okular-devel

Reply via email to