Re: [Libreoffice] [REVIEW] Complete fix for fdo#32684

Hi Michael,

On Mon, 2011-05-23 at 12:20 +0100, Michael Meeks wrote:
> On Mon, 2011-05-23 at 12:55 +0200, Cedric Bosdonnat wrote:
> > That bug was badly fixed (by me) quite some times ago. Here is a
> > correct fix (why did I add that code at all ?). Could you review it
> > and push it in 3.4 and 3.4.0?
>
>       Hmm.
>
> > More cleanup is needed but it'll end up in master: some options aren't
> > used at all in that area.
>
>       So - its hard to see quite what problem it causes; but:

The problem is fairly well described in the bug report: enable CJK
support in the options and what ever you do the rulers unit in sw will
always be set back to Char / Line (which is non-sense for all western
locales).

> ..
> > -    sal_Bool bCJKEnabled = aCJKOptions.IsAsianTypographyEnabled();
> > -    bApplyCharUnit = bCJKEnabled;
> > -    eHScrollMetric = bApplyCharUnit ? FUNIT_CHAR : eUserMetric;
> > -    eVScrollMetric = bApplyCharUnit ? FUNIT_LINE : eUserMetric;
>
>       Surely leaving un-initialized members lying around in the instance is
> worse than initializing them to wrong values ? How dangerous are the
> wrong values if they are not used ?

The bApplyCharUnit is already set to false a few lines above... and when
the H/VScrollMetric values aren't set the global unit is used (set a few
lines above too).

That patch is mostly reverting parts of this commit:
http://cgit.freedesktop.org/libreoffice/writer/commit/?id=552cdb5b8b406238486b9415262a551cb29ca817

So nothing more dangerous than what we had in previous versions.

-- 
Cédric Bosdonnat
LibreOffice hacker
http://documentfoundation.org
OOo Eclipse Integration developer
http://cedric.bosdonnat.free.fr