Bug Hunting Session
Bug 75364 - UI: Formatting of Checkbox in a Form alternated - 3D became flat and flat became 3D
Summary: UI: Formatting of Checkbox in a Form alternated - 3D became flat and flat bec...
Status: VERIFIED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: Base (show other bugs)
Version:
(earliest affected)
3.4.0 release
Hardware: Other Linux (All)
: medium normal
Assignee: Lionel Elie Mamane
URL:
Whiteboard: target:4.4.0 target:4.2.6 target:4.3.0.1
Keywords: regression
: 33557 40381 (view as bug list)
Depends on:
Blocks:
 
Reported: 2014-02-22 11:40 UTC by Robert Großkopf
Modified: 2014-06-09 16:33 UTC (History)
4 users (show)

See Also:
Crash report or crash signature:


Attachments
Database with a form, different formatted checkboxes. (11.03 KB, application/vnd.oasis.opendocument.base)
2014-02-22 11:40 UTC, Robert Großkopf
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Robert Großkopf 2014-02-22 11:40:06 UTC
Created attachment 94561 [details]
Database with a form, different formatted checkboxes.

Open the attached database.
Open the form.
There are two checkboxes. One is formatted as "3D", the other is formatted as "Flat".
Up to LO 3.3.4 it looks right.
Beginning with LO 3.4.0Beta1 "3D" is shown as "Flat" and "Flat" is shown AS "3D". It's the same wrong behavior up to LO 4.2.1.1.

My system: OpenSUSE 12.3 64bit rpm Linux.
Comment 1 Julien Nabet 2014-05-28 23:45:37 UTC
With this patch:
diff --git a/extensions/source/propctrlr/formres.src b/extensions/source/propctrlr/formres.src
index 728c784..3b479cc 100644
--- a/extensions/source/propctrlr/formres.src
+++ b/extensions/source/propctrlr/formres.src
@@ -1147,11 +1147,11 @@ Resource RID_RSC_ENUM_VISUALEFFECT
 {
     String 1
     {
-        Text [ en-US ] = "3D";
+        Text [ en-US ] = "Flat";
     };
     String 2
     {
-        Text [ en-US ] = "Flat";
+        Text [ en-US ] = "3D";
     };
 };

it seems to work. (Nevertheless, you must change property the first time so the form is synchonized with options of the properties edition).

The problem is I just tried to "guess" I got lost trying to understand how this part works. Indeed, Opengroking RID_RSC_ENUM_VISUALEFFECT and related didn't help.

Lionel: any idea?
Comment 2 Julien Nabet 2014-05-28 23:54:56 UTC
Forget my last comment, it doesn't work
Comment 3 Lionel Elie Mamane 2014-05-29 06:08:47 UTC
(In reply to comment #1)
> With this patch:
> diff --git a/extensions/source/propctrlr/formres.src
> b/extensions/source/propctrlr/formres.src
> index 728c784..3b479cc 100644
> --- a/extensions/source/propctrlr/formres.src
> +++ b/extensions/source/propctrlr/formres.src
> @@ -1147,11 +1147,11 @@ Resource RID_RSC_ENUM_VISUALEFFECT
>  {
>      String 1
>      {
> -        Text [ en-US ] = "3D";
> +        Text [ en-US ] = "Flat";
>      };
>      String 2
>      {
> -        Text [ en-US ] = "Flat";
> +        Text [ en-US ] = "3D";
>      };
>  };

That patch is swapping the labels in the property editor for "Flat" and "3D". It "works" in a sense that now the label matches the behaviour, but:

1) as you pointed out, it introduces an incompatibility with
   LibreOffice 3.3.4 and earlier.

2) if one looks at the odb file (open it as a zip file),
   in forms/Obj11/content.xml
   (use an XML pretty-printer, I use "xmlindent -f -w")
   one sees:

   <form:checkbox form:name="Check Box 1" (...)
                  form:label="Check Box 1, Style → Flat"
                  form:visual-effect="flat" (...)>

So clearly the label in the properties editor is right (Flat is flat), and it is really LibreOffice's behaviour we have to fix.

Thanks for pointing me in the right direction, though. Some "git grep -i visualeffect" later, the source of this bug is:

commit 696996791d1dbfd8f410236e30be5a1fb100de70
Author: Philipp Lohmann [pl] <Philipp.Lohmann@Oracle.COM>
Date:   Wed Nov 24 18:39:49 2010 +0100

    vcl117: #i115686# remove old unused style setting

-    static void setVisualEffect( const Any& _rValue, Window* _pWindow, void (StyleSettings::*pSetter)( USHORT ), sal_Int16 _nFlatBits, sal_Int16 _n3DBits )
+    static void setVisualEffect( const Any& _rValue, Window* _pWindow )
     {
         AllSettings aSettings = _pWindow->GetSettings();
         StyleSettings aStyleSettings = aSettings.GetStyleSettings();
 
         sal_Int16 nStyle = LOOK3D;
         OSL_VERIFY( _rValue >>= nStyle );
         switch ( nStyle )
         {
         case FLAT:
-            (aStyleSettings.*pSetter)( _nFlatBits );
+            aStyleSettings.SetOptions( aStyleSettings.GetOptions() & ~STYLE_OPTION_MONO );
             break;
         case LOOK3D:
         default:
-            (aStyleSettings.*pSetter)( _n3DBits );
+            aStyleSettings.SetOptions( aStyleSettings.GetOptions() | STYLE_OPTION_MONO );
         }
         aSettings.SetStyleSettings( aStyleSettings );
         _pWindow->SetSettings( aSettings );
     }

The SetOptions lines are "simply" swapped.
As can be seen e.g. in file svx/source/fmcomp/gridcell.cxx, STYLE_OPTION_MONO corresponds to FLAT, not to LOOK3D:

namespace
{
    void setCheckBoxStyle( Window* _pWindow, bool bMono )
    {
        AllSettings aSettings = _pWindow->GetSettings();
        StyleSettings aStyleSettings = aSettings.GetStyleSettings();
        if( bMono )
            aStyleSettings.SetOptions( aStyleSettings.GetOptions() | STYLE_OPTION_MONO );
        else
            aStyleSettings.SetOptions( aStyleSettings.GetOptions() & (~STYLE_OPTION_MONO) );
        aSettings.SetStyleSettings( aStyleSettings );
        _pWindow->SetSettings( aSettings );
    }
}

is called like that:

void DbCheckBox::Init( Window& rParent, const Reference< XRowSet >& xCursor )
{
   (...)
        setCheckBoxStyle( m_pWindow, nStyle == awt::VisualEffect::FLAT );
        setCheckBoxStyle( m_pPainter, nStyle == awt::VisualEffect::FLAT );
   (...)
}


Also, further down in commit 696996791d:

void VCLXCheckBox::setProperty( const ::rtl::OUString& PropertyName, const ::com::sun::star::uno::Any& Value) throw(...)
{
     (...)
             case BASEPROPERTY_VISUALEFFECT:
-                ::toolkit::setVisualEffect( Value, pCheckBox, &StyleSettings::SetCheckBoxStyle, STYLE_CHECKBOX_MONO, STYLE_CHECKBOX_WIN );
+                ::toolkit::setVisualEffect( Value, pCheckBox );
     (...)
}

So before this commit, _nFlatBits (which corresponds to style FLAT) is STYLE_CHECKBOX_MONO, again FLAT corresponds to MONO.
Comment 4 Lionel Elie Mamane 2014-05-29 06:14:49 UTC
*** Bug 33557 has been marked as a duplicate of this bug. ***
Comment 5 Lionel Elie Mamane 2014-05-29 06:25:16 UTC
*** Bug 40381 has been marked as a duplicate of this bug. ***
Comment 6 Julien Nabet 2014-05-29 06:38:21 UTC
Lionel: You put this tracker as FIXED but there's no commit in the core repo, did you use a branch (and the changes will be in master branch in a second time) or did I miss something? Also, I think it could be interesting to cherry-pick the fix at least for 4.3 branch.
Comment 7 Commit Notification 2014-05-29 15:53:31 UTC
Lionel Elie Mamane committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=ef530d0a2c0f68da7011f935ff4775defc6d4651

fdo#75364 flat style and 3d style were swapped



The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds
Affected users are encouraged to test the fix and report feedback.
Comment 8 Lionel Elie Mamane 2014-05-29 16:06:32 UTC
(In reply to comment #6)
> Lionel: You put this tracker as FIXED but there's no commit in the core
> repo,

Discovered another related bug while testing the duplicates, got sidelined into fixing it before pushing the two fixes together. Now done. Backport to 4-3 and 4-2 submitted to gerrit.
Comment 9 Julien Nabet 2014-05-31 07:48:11 UTC
With master sources updated today, I no longer reproduce the problem.
Thank you Lionel!
Comment 10 Commit Notification 2014-06-04 07:33:49 UTC
Lionel Elie Mamane committed a patch related to this issue.
It has been pushed to "libreoffice-4-2":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=1fd9e8e1bdf1ddf32b21d12e3793cb7a2c53e1a1&h=libreoffice-4-2

fdo#75364 flat style and 3d style were swapped


It will be available in LibreOffice 4.2.6.

The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds
Affected users are encouraged to test the fix and report feedback.
Comment 11 Commit Notification 2014-06-09 16:33:05 UTC
Lionel Elie Mamane committed a patch related to this issue.
It has been pushed to "libreoffice-4-3":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=2f59d362f9d6111270fcf58957496359ff352502&h=libreoffice-4-3

fdo#75364 flat style and 3d style were swapped


It will be available in LibreOffice 4.3.

The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds
Affected users are encouraged to test the fix and report feedback.