[Okular-devel] Review Request 109364: Get background color from KColorScheme (kde system settings)

[Albert Astals Cid]

> On March 9, 2013, 4:12 p.m., Albert Astals Cid wrote:
> > I am not sure this is the totally correct fix (I'm not saying the code we have now is better). But i do have here epubs that set the color of the text to black  ( e.g http://bugs.kde.org/attachment.cgi?id=51488 ), so in case my i would get black on black :-/
> > 
> > On the other hand i see the problem the reporter is having.
> > 
> > OTOH in further inspection of http://bugs.kde.org/attachment.cgi?id=51488 seems it is declaring it wants its background to be white, wonder why we are not getting it white then :S
> > 
> > Do yo have time to investigate why this is happening? Because it we commit this patch directly i feel it would be going from one bug to another, and not sure it makes much sense
> 
> Azat Khuzhin wrote:
>     I'm working on it, but for now the patch attached here is the simplest way to fix it.
>     Because user can change color scheme and read document (background will be changed), instead always white background.
>     
>     But for now, I'm trying to extract background from epub documents, and when I do that, I'll upload the patch.
> 
> Azat Khuzhin wrote:
>     It seems that we can't extract background color for epub documents (in particular for this one)
>     There is no information about background colors.
>     
>     We can force text color to opposite (black -> white), but, in my opinion, this is not correct.
>     Or we can rewrite epub backend not to use libepub, than, I think, we can get such information.
> 
> Albert Astals Cid wrote:
>     "user can change color scheme and read document", well they can already do that now, no?
>     
>     "There is no information about background colors." it's there in the CSS we pass down to QTextDocument, i guess the question is why that's not working. 
>     
>     Do you have time to do a simple code check with something like
>     
>     body {
>             background-color:#fff;
>     }
>     
>     in the css and see if it works? Because that's close to what we are passing down to QTextDocument.
> 
> Azat Khuzhin wrote:
>     - well they can already do that now, no?
>     Sure they do, but, the background always be white, without this patch.
>     
>     I've already tested this, and background-color property is not presented in output QTextDocument::toHtml()
> 
> Azat Khuzhin wrote:
>     epub_get_iterator() returns *.html pages, without *.css files

Have a look at EpubDocument::loadResource


- Albert


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


On March 8, 2013, 11:38 p.m., Azat Khuzhin wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/109364/
> -----------------------------------------------------------
> 
> (Updated March 8, 2013, 11:38 p.m.)
> 
> 
> Review request for Okular.
> 
> 
> Description
> -------
> 
> Instead of just force it to white.
>     
> This must fix bug 306572.
> https://bugs.kde.org/show_bug.cgi?id=306572
> 
> 
> This addresses bug 306572.
>     http://bugs.kde.org/show_bug.cgi?id=306572
> 
> 
> Diffs
> -----
> 
>   core/textdocumentgenerator.cpp f370ded 
> 
> Diff: http://git.reviewboard.kde.org/r/109364/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Azat Khuzhin
> 
>

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