[Kmymoney-devel] KMyMoney: proposal to replace hard coded utf-8 in KReportsView::KReportTab

[Alvaro Soliverez]

On Mon, Mar 29, 2010 at 3:52 PM, Bernd Gonsior <bgo at freeplexx.de> wrote:
>> Hello Bernd,
>> besides the html encoding, is there another fix or feature included in
>> your patches?
>> The encoding is already fixed in SVN, and your patch includes a
>> refactoring which is nice, but makes it harder to spot any other
>> changes in there.
>>
>> Regards,
>> Alvaro
>
> There is no other fix or feature in the patch. But there is a problem:
> Originally 'utf-8' was hard coded __twice__.
>
> As far as i can see svn revision r1107848 replaces only one of them. Currently
> (svn revision 1108741) there is still a hard coded 'utf-8' in
> kmymoney/views/kreportsview.cpp, line 147:
>
>  QRegExp exp("(.*)(<link.*css\" href=)\"(.*)\">(<meta.*utf-8\" />)(.*)");
>  QString table = createTable();
>  if (exp.indexIn(table) != -1 && includeCSS) {
>
> This code causes 2 problems:
>
> 1. in the current state the execution of the regexp 'exp.indexIn(table)' takes
> a very long time when the html generated by 'createTable()' does NOT contain
> 'utf-8' but - as it is in my case - 'ISO-8859-1'. It took 119 seconds (yes - 2
> minutes!) with 100% CPU on a Phenom II 3 GHz for a simple 2 pages html-report
>
> Obviously this regexp was used to split the html-string generated by
> 'createTable()' in a way, that the css which was included as link to a file
> could be replaced with inline-css, if necessary.
>
> 2. minor problem:
> I do not know how the compiler optimizes the logical AND in
> 'exp.indexIn(table) != -1 && includeCSS'. If the compiler simply works
> left-to-right, the expensive regexp will be evaluated always.
>
> When first grepping the source for 'utf-8' it looked like a simple problem,
> but it emerged a developing story as mentioned above. That's why i thought
> replacing that implicit regexp with explicit declared methods could help.
>
> The changes compared to the original state before svn revision r1107848 at a
> glance:
>
> - all changes occured in KReportsView::KReportTab only
> - 1 method signature was changed:
>  - createTable(const QString& links, bool includeCSS)
>  (the method has a new parameter 'includeCSS' with default value 'false')
> - 2 method bodies were changed:
>  - saveAs
>  - createTable
> - 3 new methods were introduced:
>  - cssFileNameGet
>  - htmlHeaderGet
>  - htmlFooterGet
>
> HTH
> Bernd
>

Ok. I'll fix the other encoding. I didn't spot that one.

Now, for a more permanent fix of this issue, I'd like to move the
whole HTML generation code to a separate class. That would move a lot
of code which doesn't really belong to the view out of the main class.
Would you like to take on that?

Regards,
Alvaro