Bug 114572 - UI: Backspace/ delete isn't working as expect for the table column width fields (persistently add a size)
Summary: UI: Backspace/ delete isn't working as expect for the table column width fiel...
Status: RESOLVED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: UI (show other bugs)
Version:
(earliest affected)
6.0.0.0.alpha1+
Hardware: All All
: medium normal
Assignee: Caolán McNamara
URL:
Whiteboard: target:6.1.0 target:6.0.0.2
Keywords: bibisected, bisected, regression
Depends on:
Blocks:
 
Reported: 2017-12-19 20:51 UTC by Telesto
Modified: 2018-01-03 16:15 UTC (History)
2 users (show)

See Also:
Crash report or crash signature:


Attachments
Bibisect log (3.07 KB, text/plain)
2017-12-19 21:00 UTC, Telesto
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Telesto 2017-12-19 20:51:32 UTC
Description:
UI: Backspace/ delete isn't working as expect for the table column width fields (persistently add a size)

Steps to Reproduce:
1. Open the attached file
2. Right Click -> Table Properties
3. Column Tab
4. Set the cursor after CM in any column size field and old backspace. 29,54 will be added.
5. Place the cursor at the beginning; press delete: it will contain 0,94 cm

Actual Results:  
Changing the size manually be replacing a number (by deleting one first) is impossible

Expected Results:
Should be possible, as before


Reproducible: Always


User Profile Reset: No



Additional Info:
Found in
Version: 6.1.0.0.alpha0+
Build ID: aad9c6da5154a89c6ef02214d1122d4b444eea23
CPU threads: 4; OS: Windows 6.3; UI render: default; 
TinderBox: Win-x86@42, Branch:master, Time: 2017-12-15_22:56:44
Locale: nl-NL (nl_NL); Calc: group threaded

and in
Version: 6.0.0.0.alpha1+
Build ID: 71c497d0eff0fc4179f31512c75029c48a1c9407
CPU threads: 4; OS: Windows 6.3; UI render: default; 
Locale: nl-NL (nl_NL); Calc: CL

but not in
Version: 5.4.0.3
Build ID: 7556cbc6811c9d992f4064ab9287069087d7f62c
CPU threads: 4; OS: Windows 6.2; UI render: default; 
Locale: nl-NL (nl_NL); Calc: CL


User-Agent: Mozilla/5.0 (Windows NT 6.3; WOW64; rv:52.0) Gecko/20100101 Firefox/52.0
Comment 1 Telesto 2017-12-19 21:00:03 UTC
Created attachment 138535 [details]
Bibisect log

Bisected to:

author	Caolán McNamara <caolanm@redhat.com>	2017-09-13 21:02:32 +0100
committer	Caolán McNamara <caolanm@redhat.com>	2017-09-14 14:40:25 +0200
commit 44bfe8fad4f7c263dc713a65fb2ab0e2f9afcf99 (patch)
tree 1a75093344dc2ee8e6d5bd7c21e3dd8dd4451596
parent 34ac0f9a0376b43bcff78a49ccaf4caa34c8c990 (diff)
connect to modified instead of up/down/focus-lost
Comment 2 Aron Budea 2017-12-21 17:15:45 UTC
There's no attached sample, but it is easy to see that deleting characters in the column width spinfield produces interesting results.
What happens is that after each change the current entry is interpreted as the new number, and then clamped to [min, max] values.

Let's say, with a 2x2 table, each column is 8.50 cm.
Put cursor behind 8, hit backspace. => Entry becomes 0.50 cm (correctly).
Enter 9. => Entry is interpreted as 90.50 cm, and restricted to the maximum, which is 16.96 cm (at least with A4 and default margins).

Caolán, what was the reason behind the change in the commit?
I can see that in previous version if the field was changed using backspace/delete, and then confirmed with OK, the change didn't register (interestingly it did when the numbers are selected, and then overwritten).
Comment 3 Caolán McNamara 2017-12-22 10:38:14 UTC
"what was the reason behind the change in the commit?"

Yeah, well, the real reason is that these are groundworks for moving to native widgets so trying to align our current implementation with how other widget frameworks work so that changeover would be smooth.

"I can see that in previous version if the field was changed using backspace/delete, and then confirmed with OK, the change didn't register"

Yeah, the old scheme was to update when up/down is clicked, or when the widget loses focus and or various other events. Moving to immediate update seemed like a good idea to achieve the same results and avoid that issue that the number doesn't "stick" if you don't move to another widget first to get the focus-lost signal that you see there in the older version.

Anyway, lets revert for now to go back to the old way and I'll try a different approach.
Comment 4 Commit Notification 2017-12-22 19:09:07 UTC
Caolán McNamara committed a patch related to this issue.
It has been pushed to "master":

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

Resolves: tdf#114572 immediate update of value while typing unwanted

It will be available in 6.1.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 5 Telesto 2018-01-01 12:04:51 UTC
@Caolán
This one should be backported to 6.0 branch, I suppose..
Comment 6 Commit Notification 2018-01-03 16:15:00 UTC
Caolán McNamara committed a patch related to this issue.
It has been pushed to "libreoffice-6-0":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=ccce3d15fdb28786166f9c96cdc12a185c3c078e&h=libreoffice-6-0

Resolves: tdf#114572 immediate update of value while typing unwanted

It will be available in 6.0.0.2.

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.