Bug 125054 - Font changed when typing into cell, in edit mode, with redline
Summary: Font changed when typing into cell, in edit mode, with redline
Status: VERIFIED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: Calc (show other bugs)
Version:
(earliest affected)
6.3.0.0.alpha0+
Hardware: All All
: highest critical
Assignee: Armin Le Grand (allotropia)
URL:
Whiteboard: target:6.3.0
Keywords: bibisected, bisected, regression
: 125092 125140 125305 125617 125654 (view as bug list)
Depends on:
Blocks:
 
Reported: 2019-04-30 20:14 UTC by Roman Kuznetsov
Modified: 2019-06-10 20:16 UTC (History)
8 users (show)

See Also:
Crash report or crash signature:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Roman Kuznetsov 2019-04-30 20:14:18 UTC
Description:
It changes font name when types anything into cell

Steps to Reproduce:
1. Open Calc with clean user profile
2. Select A1 (font name is Liberation Sans)
3. Try type anything into cell and don't press Enter
4. See on font name -> it's Liberation Serif!
5. Press Enter and select A1 cell
6. Font name is Liberation Sans!

Actual Results:
It changes font name when types anything into cell

Expected Results:
Doesn't change font name when types anything into cell


Reproducible: Always


User Profile Reset: No



Additional Info:
Version: 6.3.0.0.alpha0+ (x64)
Build ID: db39336550ff547bcb7ca15793b12291c913ab42
CPU threads: 4; OS: Windows 10.0; UI render: default; VCL: win; 
TinderBox: Win-x86_64@42, Branch:master, Time: 2019-04-29_23:46:36
Locale: ru-RU (ru_RU); UI-Language: en-US
Calc: threaded

but not in LO 6.2.3 -> regression

I bisected it:

$ git bisect good 8ebd3512f356183eaab4c0f6281bcf1968df48ca is the first bad commit
commit 8ebd3512f356183eaab4c0f6281bcf1968df48ca
Author: Norbert Thiebaud <nthiebaud@gmail.com>
Date:   Thu Apr 25 11:48:50 2019 -0700

    source 1e2682235cded9a7cd90e55f0bfc60a1285e9a46


https://gerrit.libreoffice.org/#/c/71075/

CC: to Armin Le Grand
Comment 1 Jean-Baptiste Faure 2019-05-01 15:59:09 UTC
I see the same thing in Version: 6.3.0.0.alpha0+
Build ID: f7453b956bcf83ec13c805d243f20cb209289179
Threads CPU : 4; OS : Linux 4.15; UI Render : par défaut; VCL: gtk3; 
Ubuntu_18.04_x86-64
Locale : fr-FR (fr_FR.UTF-8); Langue IHM : fr-FR
Calc: threaded

Status set to NEW.

Best regards. JBF
Comment 2 Aron Budea 2019-05-02 12:11:01 UTC
Let's reprioritize, as it's a broken core function and a major component affecting most users.
Comment 3 Xisco Faulí 2019-05-06 16:21:16 UTC
*** Bug 125140 has been marked as a duplicate of this bug. ***
Comment 4 Xisco Faulí 2019-05-07 11:48:36 UTC
*** Bug 125092 has been marked as a duplicate of this bug. ***
Comment 5 Armin Le Grand (allotropia) 2019-05-15 08:55:33 UTC
Sorry for late reaction, but I got *no* eMail which I usually use as 'trigger' to take a look - had changed all eMail addresses to my private one, but does not work for some reason.
Comment 6 Armin Le Grand (allotropia) 2019-05-15 14:20:13 UTC
I think I have found the source for the error: The before used ::operator=() of SfxPoolItems did *not* copy the WhichID. There were places like in ScPatternAttr::FillToEditItemSet which prepared a new Item which is now

    std::shared_ptr<SvxColorItem> aColorItem(std::make_shared<SvxColorItem>(EE_CHAR_COLOR));              // use item as-is

then used ::operator=() to copy something to it which is now

        aColorItem.reset(static_cast<SvxColorItem*>(pItem->Clone()));

and then used it in calls to e.g.

        rEditSet.Put( *aColorItem, EE_CHAR_COLOR );

which in effect *changed* the WhichID of the item. The ::Clone() call to the SfxPoolItem which is used now to work with an instance of the Item will - of course - also clone the WhichID.

Thus the up-to-now indirectly 'used' functionality is that the ::operator=() did *not* change the evtl. already set WhichID of the target item. This is usage of very hidden functionality, it would be perfectly OK to assume that ::operator= copies everything and thus is equal in that respect to ::Clone() - but it is *not*.

Checking my changes for that aspect...
Comment 7 Armin Le Grand (allotropia) 2019-05-15 14:23:56 UTC
Upps - comment above: Usage of

    rEditSet.Put( *aColorItem, EE_CHAR_COLOR );

is the already fixed behaviour - use new target-WhichID to put item. Putting item will check if item is there and anyways clone it (and then adapt the WhichID - that seems to be the historical reason for that ::Put call form ...

Hint: It will be good to get that Item stuff type-safe and get rid of WhichIDs completely - as this example shows all kinds of hidden effects are associated with these...
Comment 8 Armin Le Grand (allotropia) 2019-05-15 16:14:58 UTC
Proposed fix on gerrit (https://gerrit.libreoffice.org/#/c/72372/)
Comment 9 Commit Notification 2019-05-16 15:43:08 UTC
Armin Le Grand committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/+/7ed69df31bb5f3f4e89ca2ef914e103676836362%5E%21

tdf#125054 fixed WhichIDs for cloned Items

It will be available in 6.3.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 10 Armin Le Grand (allotropia) 2019-05-16 15:43:39 UTC
Fix committed to master
Comment 11 Xisco Faulí 2019-05-17 11:09:10 UTC
Verified in

Version: 6.3.0.0.alpha1+
Build ID: 4c2034b808fed4f9dfd715d8a4813e788a7e97a4
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

@Armin Le Grand, thanks for fixing this issue!!!
Comment 12 Stefan_Lange_KA@T-Online.de 2019-05-17 19:38:20 UTC
Also tested with the test document from Bug 125092 with 
Version: 6.3.0.0.alpha1+ (x64)
Build ID: 55961e110f0bf2d571a27e56806dae50fa353233
CPU threads: 4; OS: Windows 10.0; UI render: GL; VCL: win; 
TinderBox: Win-x86_64@42, Branch:master, Time: 2019-05-17_07:52:09
Locale: de-DE (de_DE); UI-Language: en-US

Result: Test was OK, many thanks!
Comment 13 Roman Kuznetsov 2019-05-18 20:11:56 UTC
*** Bug 125305 has been marked as a duplicate of this bug. ***
Comment 14 Lars Jødal 2019-06-04 06:23:09 UTC
*** Bug 125654 has been marked as a duplicate of this bug. ***
Comment 15 Xisco Faulí 2019-06-10 20:16:18 UTC
*** Bug 125617 has been marked as a duplicate of this bug. ***