Bug 99711 - Sidebar Position and Size displays incorrect width and height when units in mm
Summary: Sidebar Position and Size displays incorrect width and height when units in mm
Status: VERIFIED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: Draw (show other bugs)
Version:
(earliest affected)
4.4.0.3 release
Hardware: All All
: highest major
Assignee: Caolán McNamara
URL:
Whiteboard: target:6.5.0 target:6.4.0.1 target:6....
Keywords: bibisected, bisected, regression
: 107118 107878 115322 118837 124447 (view as bug list)
Depends on:
Blocks: Sidebar-Properties-PositionAndSize
  Show dependency treegraph
 
Reported: 2016-05-06 14:12 UTC by menturi
Modified: 2020-05-20 08:20 UTC (History)
16 users (show)

See Also:
Crash report or crash signature:


Attachments
Step 2 (92.13 KB, image/png)
2016-05-06 14:12 UTC, menturi
Details
Step 3 (92.56 KB, image/png)
2016-05-06 14:13 UTC, menturi
Details
output from bibisect in 43max repository (5.12 KB, text/plain)
2017-01-09 19:30 UTC, Terrence Enger
Details
The evidence in LO 5.1.6.2 (298.62 KB, image/png)
2017-05-28 02:02 UTC, Emersson Augusto Suarez Ortiz
Details
Problem on 5.3.4 (53.57 KB, image/png)
2017-07-09 10:59 UTC, RGB
Details

Note You need to log in before you can comment on or make changes to this bug.
Description menturi 2016-05-06 14:12:33 UTC
Created attachment 124883 [details]
Step 2

Problem description:
The sidebar displays incorrect width and height when units are in millimeters

Steps to reproduce:
1. Open LibreOffice Draw and set the options to display mm (Tools > Options > LibreOffice Draw > General > Unit of measurement: Millimeter)
2. Create a rectangle and change the width and height to 10.00 mm. Note that the Position and Size window* agrees with the sidebar for these dimensions. *right click on the rectangle > Position and Size
3. Deselect and reselect the rectangle. The sidebar now claims the width and height are 9.91 mm, however the Position and Size window correctly shows 10.00 mm.

Note that if a 10.00 mm grid is shown, the 10.00 mm rectangle aligns to the grid, whereas a 9.91 mm rectangle does not. This suggests the dimensions are actually 10.00 mm, even though the sidebar displays them as 9.91 mm.

Current behavior:
In the Sidebar, under Position and Size, the Width and Height display incorrect width and height.

Expected behavior:
In the Sidebar, under Position and Size, the Width and Height should display the correct width and height.

Additional comments:
-Tested on a Windows 7 (x64) machine using LibreOffice 5.0.5.2
-Able to be produced this issue in both LibreOffice Draw and Impress
-Though untested, I expect similar behavior when other units are used. This may be a rounding issue before unit conversions?
Comment 1 menturi 2016-05-06 14:13:02 UTC
Created attachment 124884 [details]
Step 3
Comment 2 Buovjaga 2016-05-07 13:21:49 UTC
Repro.

menturi: would you like to join the QA team to do this: https://wiki.documentfoundation.org/QA/Triage_For_Beginners

Arch Linux 64-bit, KDE Plasma 5
Version: 5.2.0.0.alpha1+
Build ID: 540fee2dc7553152914f7f1d8a41921e765087ef
CPU Threads: 8; OS Version: Linux 4.5; UI Render: default; 
Locale: fi-FI (fi_FI.UTF-8)
Built on April 30th 2016
Comment 3 Nico Stuurman 2017-01-04 01:10:14 UTC
This bug is not present in Libreoffice 4.1.0.4 (the first version with a sidebar), thus it's a regression

Adding "regression" to the keywords
Comment 4 Buovjaga 2017-01-04 08:29:32 UTC
Thanks for the test, Nico. Adding bibisectRequest.
Comment 5 raal 2017-01-06 05:21:54 UTC
repro Version: 4.4.0.0.alpha0+
Comment 6 Terrence Enger 2017-01-09 19:30:12 UTC
Created attachment 130286 [details]
output from bibisect in 43max repository

Working on debian-stretch in the bibisect-43max repository, I see that
the bug entered LibreOffice in commit 36be3d94 ...

    Author:     Armin Le Grand <alg@apache.org>
    AuthorDate: Thu Mar 20 14:36:21 2014 +0000
    Commit:     Caolán McNamara <caolanm@redhat.com>
    CommitDate: Thu Mar 20 16:49:46 2014 +0000

        Resolves: #i124409# use slot SID_ATTR_METRIC...
    
        to retrive the UI unit, not GetModuleFieldUnit
    
        (cherry picked from commit 34279ea85c33e3efd21971ab692a3de4bdd91817)
    
        Conflicts:
                svx/source/sidebar/possize/PosSizePropertyPanel.cxx
    
        Change-Id: Id81847bf7e989a3e49fbe8adaad23048956067df

Looking at a couple of numbers, it seems that the redisplayed
dimension is converted to nearest 0.01 inch and that result is
converted to the nearest 0.01 millimetre.

I am removing keyword bibisectRequest and adding bibisected and bisected.
Comment 7 Xisco Faulí 2017-01-09 21:25:40 UTC
Commit hash: 36be3d94c2e142d01c026a93fa88454cb5316bff
Adding Cc: to Armin Le Grand
Comment 8 file-a-bug 2017-04-12 14:34:16 UTC
*** Bug 107118 has been marked as a duplicate of this bug. ***
Comment 9 Buovjaga 2017-05-19 17:11:46 UTC
*** Bug 107878 has been marked as a duplicate of this bug. ***
Comment 10 Emersson Augusto Suarez Ortiz 2017-05-28 02:01:03 UTC
The size is displayed incorrect and no matter which unit do you use. I have LO 5.1.6.2 in Linux Mint and I confirmed this bug.
Comment 11 Emersson Augusto Suarez Ortiz 2017-05-28 02:02:14 UTC
Created attachment 133658 [details]
The evidence in LO 5.1.6.2
Comment 12 RGB 2017-07-09 10:59:08 UTC
Created attachment 134557 [details]
Problem on 5.3.4

As you can see on the screenshot the problem is also present when you select cm as unit, not only with mm. 

If you take the quotient between the value displayed on the menu and the one on the sidebar you get ~2.5. It seems the value displayed on the sidebar is on inches, even if it says cm.
Comment 13 QA Administrators 2018-07-10 02:36:55 UTC Comment hidden (obsolete)
Comment 14 RGB 2018-07-10 16:25:02 UTC
Problem still present in  6.0.5.2
Comment 15 CassieLX 2018-07-25 11:44:28 UTC
I can still confirm this bug in linux and lo 6.0.5.2. This is very anoying, because it changes the sizes of objects in the document. And objects I didn't touch change their sizes and dimension lines too. For example 60cm becomes 62,59 and I didn't touch this drawing object. This happens while I changed another objects on this page.
Comment 16 CassieLX 2018-07-25 12:04:49 UTC
It'seems to work, if you DON'T TOUCH THE SIDEBAR.
Comment 17 RGB 2018-08-31 18:33:02 UTC Comment hidden (obsolete)
Comment 18 RGB 2018-08-31 18:54:08 UTC
OK, it doesn't work: for some reason, it started to work when using some drawing objects, then I selected a PNG picture and it failed, now even the drawing objects show different values in the sidebar and in the "Position and size" menu. (I tested on 6.1.1.1)
Comment 19 Buovjaga 2018-09-19 19:14:45 UTC
*** Bug 118837 has been marked as a duplicate of this bug. ***
Comment 20 Buovjaga 2018-09-19 19:15:48 UTC
*** Bug 115322 has been marked as a duplicate of this bug. ***
Comment 21 V Stuart Foote 2019-03-30 17:19:30 UTC
*** Bug 124447 has been marked as a duplicate of this bug. ***
Comment 22 kyumin.lee 2019-04-08 15:53:59 UTC
I confirm that the bug is present in the version 6.2.2.2 as well. Very annoying.
Comment 23 Xisco Faulí 2019-12-02 11:47:09 UTC Comment hidden (obsolete)
Comment 24 Xisco Faulí 2019-12-02 11:47:46 UTC
@Caolán, I thought you might be interested in this issue...
Comment 26 Xisco Faulí 2019-12-02 13:40:47 UTC
Changing priority to 'highest' since this a regression and the number of duplicates is higher than 5 or the number of people in CC higher than 20
Comment 27 Roland Illig 2019-12-02 14:56:01 UTC
When fixing this bug, it might be a good idea to add a unit test, to prevent it from being broken ever again.

As I looked around in PosSizePropertyPanel.cxx, I noticed that the spacing in the code seems to be completely random. I found:

* if(expr)
* if (expr)
* if( expr )
* if ( expr )

* call(arg1, arg2)
* call(arg1,arg2)
* call( arg1, arg2 )
* call( "Width")

I would expect that a project as large as LibreOffice has a style guide for these things.

In the "nChar == '-'" around line 496, the left-hand side is a modifiable expression. The other code in that file uses the "if (object == subject)" style to avoid this, although this typo is nowadays detected by any good compiler.

The number 315000 looks like a typo to me since it has 1 more 0 than expected. The value 31500 makes more sense when measured in 1/100ths of degrees. By the way, nTmp is a terrible variable name. I suggest nAngle instead.

All in all, it looks like there is a lot to do in this single file.
I haven't looked at the other files, I guess they look similar.
Comment 28 Commit Notification 2019-12-03 21:15:57 UTC
Caolán McNamara committed a patch related to this issue.
It has been pushed to "libreoffice-6-3":

https://git.libreoffice.org/core/commit/162a9d7e625315bb7d2f7353bfe160dc2217defa

Resolves: tdf#99711 update metric for spinbuttons before setting their values

It will be available in 6.3.5.

The patch should be included in the daily builds available at
https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
https://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 29 Commit Notification 2019-12-04 08:55:22 UTC
Caolán McNamara committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/704dc5814a26f56c67da4e0b947abab452c4e94d

Resolves: tdf#99711 update metric for spinbuttons before setting their values

It will be available in 6.5.0.

The patch should be included in the daily builds available at
https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
https://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 30 Roland Illig 2019-12-04 09:43:35 UTC
Do you plan to add a regression test so that the order of these method calls will not be accidentally reverted?
Comment 31 Commit Notification 2019-12-04 10:39:27 UTC
Caolán McNamara committed a patch related to this issue.
It has been pushed to "libreoffice-6-4":

https://git.libreoffice.org/core/commit/ad080296feb5663c0917816229eeb81683fc4c66

Resolves: tdf#99711 update metric for spinbuttons before setting their values

It will be available in 6.4.0.1.

The patch should be included in the daily builds available at
https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
https://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 32 Xisco Faulí 2019-12-04 12:11:52 UTC
Verified in

Version: 6.5.0.0.alpha0+
Build ID: 8f7010eb47119a2428b77f5d79fc8577d9914958
CPU threads: 4; OS: Linux 4.15; UI render: default; VCL: gtk3; 
Locale: ca-ES (ca_ES.UTF-8); UI-Language: en-US
Calc: threaded

@Caolán, thanks for fixing this issue!!
Comment 33 Commit Notification 2019-12-04 16:33:53 UTC
Caolán McNamara committed a patch related to this issue.
It has been pushed to "libreoffice-6-3-4":

https://git.libreoffice.org/core/commit/154ab3d8a265c86ba5211578757542f87478b12e

Resolves: tdf#99711 update metric for spinbuttons before setting their values

It will be available in 6.3.4.

The patch should be included in the daily builds available at
https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
https://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 34 Commit Notification 2020-05-19 23:05:45 UTC
Xisco Fauli committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/730c50946e088e7a93d9c0809444f74502e1b4b8

tdf#tdf99711: sw: Add UITest

It will be available in 7.0.0.

The patch should be included in the daily builds available at
https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
https://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.