Thorsten Behrens píše v Pá 18. 03. 2011 v 13:29 +0100:
> Christina Roßmanith wrote:
> > first bug fix. Built successfully and could open testdoc.html correctly.
Congratulations! I am looking forward to see more fixes from you.
> Very cool, checked that - it's pushed! :)
Heh, I have tried the patch yesterday and did not see any difference in
the HTML import. I was not sure what it actually fixed, so I asked
Cedric for more details (he created the simplified test case). I havn't
got the response yet.
Note that you did the hard job with finding the right place to fix, ...
Just please excuse my nitpicking. I would suggest 3 changes:
1. IMHO, it should check for the size of aToken to avoid a potential
crash:
- if ('/' == aToken.GetChar(aToken.Len()-1)) {
+ if (aToken.Len() >= 1 && '/' == aToken.GetChar(aToken.Len()-1)) {
2. #i34666# associates OOo isusezilla. The bug is evidently from Free
Desktop bugzilla, so there should be fdo#34666 instead of #i34666# in
the comment.
3. I prefer commit messages that gives a rough idea about what has been
fixed. The bug number is important but it is not that helpful when you
look a fix in a specific area. Next time, It would suggest somethig
like:
--- cut ---
comment import problems with unsupported HTML tag (fdo#34666)
--- cut ---
I have done the first two suggested changes by
http://cgit.freedesktop.org/libreoffice/libs-gui/commit/?id=95e301d0a55b8145ce0cdb037e438bbcfcd4a4eb
Best Regards,
Petr
_______________________________________________
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/libreoffice
