Bug 82396 - Sidebar: Incorrect behavior regarding real-time update of spinbox values
Summary: Sidebar: Incorrect behavior regarding real-time update of spinbox values
Status: RESOLVED WORKSFORME
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: Impress (show other bugs)
Version:
(earliest affected)
4.2.6.2 release
Hardware: All All
: medium normal
Assignee: Not Assigned
URL:
Whiteboard: target:5.2.0
Keywords:
Depends on:
Blocks: Sidebar-UI-UX Sidebar-Properties-PositionAndSize
  Show dependency treegraph
 
Reported: 2014-08-09 19:47 UTC by freddi34
Modified: 2020-10-01 23:09 UTC (History)
9 users (show)

See Also:
Crash report or crash signature:


Attachments
strace for the crash (7.26 MB, text/x-log)
2015-12-22 18:15 UTC, Jérôme Borme
Details

Note You need to log in before you can comment on or make changes to this bug.
Description freddi34 2014-08-09 19:47:54 UTC
In Impress, when an image is selected, one can change its size either by right-clicking → dialog "Position and Size", or directly in the properties sidebar. While the dialog requires confirmation through the "Ok" button, the sidebar applies changes in real-time while typing. This is a problem when interim values are wrong or unreasonable.

Example: 
The photo has a width of 27.99cm and you want to change it to 28cm. When clicking somewhere into the text field, the text cursor might appear at:
  27.99|cm
While backspacing, an interim value is:
  27.|cm
LibreOffice autocompletes to:
  27.|00cm
…while you are continuing to backspace you get for a moment:
  27|00cm
This size is very likely unreasonable, and moreover the real-time update to the photo can cause LibreOffice to freeze, crash, or even crash the user session.

Ubuntu: 14.04
LibreOffice: 4.3.1.0.0+
Build ID: 764fdb983d2fd0e24f56db987eba307ae5ae6eea
Comment 1 ign_christian 2014-08-14 15:50:06 UTC
I don't reproduce crash, but I can confirm such behavior regarding that real time update. 

Tested with LO 4.2.6.2 and 4.3.1.1 - Ubuntu 12.04 x86

Interestingly AOO 4.1.0 set max limit to 99,99 cm in Width & Height. So it will always immediately reset to 99,99 if we accidentaly make the real time update generates values over 99,99.
Comment 2 Jérôme Borme 2014-12-14 21:21:43 UTC
I have systematic crash when resizing an image to large sizes using the sidebar. In my experience anything above 1000 cm will cause a crash.

This does not happen when using the "Position and style" popup dialog because the latter will process the value before applying it, limiting the size to an upper bound (36 cm in my experience in Impress). 

Sidebar should check for the same limit as "Position and style" dialog does, and refrain from updating the screen if the entered value goes beyond acceptable limits.
Comment 3 Jérôme Borme 2014-12-14 21:24:13 UTC
Forgot the detail, I'm using LO 4.3.4.1 compiled on Gentoo linux.
Comment 4 QA Administrators 2015-12-20 16:07:16 UTC Comment hidden (ignore, obsolete)
Comment 5 Jérôme Borme 2015-12-21 17:12:22 UTC
This bug is still present in LibreOffice 5.0.4.2 (gentoo linux).

If size is 10000 cm in the side bar, LO crashes. Console output:

W: Unknown node under /registry/extlang: deprecated
X-Error: BadDrawable (invalid Pixmap or Window parameter)
	Major opcode: 62 (X_CopyArea)
	Resource ID:  0x0
	Serial No:    96486 (96486)
These errors are reported asynchronously,
set environment variable SAL_SYNCHRONIZE to 1 to help debugging
Application Error
Comment 6 Jérôme Borme 2015-12-21 17:15:09 UTC
Also, if size is set to 0, then aspect ratio is forgotten. This actually happens, it's as easy as removing first the width value before putting the final one. (Removing is same as to set width to zero.) When putting the final width value, the image becomes square.
Comment 7 Jérôme Borme 2015-12-22 18:15:11 UTC
Created attachment 121503 [details]
strace for the crash

After compiling with debug info activated I could get this warning on the console before it crashed:

warn:vcl:24775:1:vcl/unx/x11/xlimits.cxx:23: overlarge pixmap: 34772 x 34771

Complete strace attached. Corresponds to the following:
1. open any existing document that contains an image
2. in the sidebar, change size to 10000 cm
3. wait 1-2 seconds. It then crashes.

(I could not get useful backtraces yet.)
Comment 8 Commit Notification 2016-02-12 09:06:30 UTC
Rishabh committed a patch related to this issue.
It has been pushed to "master":

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

tdf#82396:Incorrect behavior on update of Size values in sidebar

It will be available in 5.2.0.

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 9 Xisco Faulí 2016-09-15 21:18:19 UTC
Hi,
Is this bug fixed?
If so, could you please close it as RESOLVED FIXED?
Regards
Comment 10 Yousuf Philips (jay) (retired) 2016-12-20 12:34:20 UTC
The fix provided does set a limit to the maximum values that can be entered, so to some degree, which definitely is better than how it was previously, but i think a better fix would be process the input values only after the spinbox controls loose focus.

@Heiko, @Stuart, @Cor, @Maxim: What are your thoughts?

Version: 5.3.0.0.beta1+
Build ID: a1f6159db30e2463b118c1571bb01a09356b7c49
CPU Threads: 2; OS Version: Linux 3.19; UI Render: default; VCL: gtk2; Layout Engine: new; 
TinderBox: Linux-rpm_deb-x86_64@70-TDF, Branch:libreoffice-5-3, Time: 2016-12-08_00:34:06
Locale: en-US (en_US.UTF-8); Calc: group
Comment 11 V Stuart Foote 2016-12-20 15:07:45 UTC
(In reply to Yousuf Philips (jay) from comment #10)

> but i think a better fix would be process the input values only after the
> spinbox controls loose focus.
> 

rescaling the full image with each advance of the spin button is not ideal, so for better response option might be to show just the outline of the image (or its frame), otherwise agree to not perform dynamic scaling--i.e. no action until the control looses focus.
Comment 12 Maxim Monastirsky 2016-12-20 21:35:09 UTC
(In reply to Yousuf Philips (jay) from comment #10)

Yes, agree that real-time update is bad. It also creates useless undo steps (suppose I try to enter "20", it will create one step for "2", and another one for "20"). There is similar problem with other sidebar panels (Image, Paragraph).

> but i think a better fix would be process the input values only after the
> spinbox controls loose focus.
In addition pressing enter should also apply the values. Or maybe we should just move the focus back to the document, like we do for font name and size boxes.
Comment 13 Yousuf Philips (jay) (retired) 2016-12-21 10:46:02 UTC
(In reply to V Stuart Foote from comment #11)
> rescaling the full image with each advance of the spin button is not ideal

I think rescaling or repositioning the image when a user presses the up and down buttons of the spinbox with their mouse is fine.

(In reply to Maxim Monastirsky from comment #12)
> Yes, agree that real-time update is bad. It also creates useless undo steps
> (suppose I try to enter "20", it will create one step for "2", and another
> one for "20"). There is similar problem with other sidebar panels (Image,
> Paragraph).

Yes correct the issue seems to happen with all sidebar spinboxes like the spacing and indent spinboxes in the paragraph content panel and the transparency slider & spinbox in area content panel. Just another reason why i believe spinboxes arent good for the sidebar.

> In addition pressing enter should also apply the values.

Yes pressing enter would be another good choice for confirming the value, though pressing enter in sidebar control's has problems (bug 98505).

> Or maybe we should
> just move the focus back to the document, like we do for font name and size
> boxes.

There are advantages and disadvantages of having enter move focus back to the document, so that would need to further discussed to have a streamline behaviour.
Comment 14 Jérôme Borme 2016-12-21 11:32:25 UTC
Actually image resizing has nothing specific in the sidebar. I personally would appreciate very much a situation where all the numerical controls of the sidebar would only be applied only when loosing focus, because similar UI problems appear with many of the sidebar controls.

For example if you want to rotate a drawing by -45 degrees, you press hyphen, then 4, then 5. If you are fast it works as expected. If you are slow: you press -4, which is converted to 356 degrees (35|6.00), then you press 5 it gives 355|6.00, then converted 316 degrees.

Same problem if you are in a position box and you want to type units. Say you want to set a size to 5 mm. If you are too slow it takes 5 m (five meters), which is resized to 23 cm, and then you are too late for the second m.

Additionally, as mentioned these cause unnecessary steps in the event history. If you modify to the size of (for example) 3 elements then decide to undo the three operations, you must press Undo a number of times: 3 x the_number_of_digits_in_the_value. This is also unexpected.

These are small bits of user frustration that come from the behavior of the sidebar which interprets immediately the user input. Actually it is not even immediate, there is an arbitrary delay of a couple hundred milliseconds, which results in unpredictable results depending on user typing abilities.
Comment 15 Xisco Faulí 2017-07-13 10:23:42 UTC
Setting Assignee back to default. Please assign it back to yourself if you're
still working on this issue
Comment 16 QA Administrators 2018-07-14 02:47:50 UTC Comment hidden (obsolete)
Comment 17 Jérôme Borme 2018-07-14 14:13:20 UTC
(In reply to QA Administrators from comment #16)

The bug is present with 6.0.5.2.

If user makes a 1 cm grouped drawing, an later wishes to resize it to 0.5 cm using the spinbox entering controls, the group drawing is *destroyed* (completely messed up). When the user enters 0.5, they press first 0, which sets all its dimensions to zero, losing their values and ratios, a situation that the drawing will not recover from when the user finishes entering 0.5.

Possible solutions (to be implemented)
* while the spindle keeps the focus, only update after a certain delay (e.g. 1 s) to let the user finish its entry.
* make immediate update (and/or the delay mentioned above) a configurable option in the preferences.
Comment 18 QA Administrators 2019-10-29 03:29:54 UTC Comment hidden (obsolete)
Comment 19 Jérôme Borme 2019-10-29 10:08:14 UTC
LibreOffice 6.3.2.2
The particular example of the reporter seems to be better in part of the cases which show the bug.
The initial report that deleting the dot in 27.|00cm to 27|00cm will crash, does not happen on my system, LibreOffice will limit the size to a value close to 50cm. However: this value is not well established, trying a few time gives on the same image values like 49,63cm, 51,40cm, 54,95cm.
So no crash, but still buggy behaviour.

The second set of test conditions still works: any size -> 0,50cm. One selects the size, then types 0,5 inside the field. Result: aspect ratio set to 1:1, and Undo will not restore the aspect ratio.
Comment 20 Jérôme Borme 2020-10-01 23:09:11 UTC
I think it is (almost almost) solved in 7.0.1.2.

- The image size in the sidebar are not anymore live updated, removing most of the problem (entry is now only applied at Enter).

- If a size above reasonable is entered, an upper value is assumed (486 cm in my case).

- if the size entered by the user is larger than the page, the effective size of the image is limited to the page, but the size desired by the user is kept in the sidebar, and the image is allowed to increase and reduce in effective size if the user changes the page size. Not a bad idea at all!

- undo now restores the aspect ratio in case a mistake is made.

- The only unsolved case is if the user enters a very small value for the image width, lets LO update the height according to aspect ratio, then the aspect ratio is not anymore correct due to rounding the height with a finite accuracy (0.01 cm). If the user changes the size back to original value, the wrong aspect ratio is scaled up. This is less of a problem because Undo now works.