Bug 154884 - Favorite Special characters: add multiple entries of the same character
Summary: Favorite Special characters: add multiple entries of the same character
Status: RESOLVED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: UI (show other bugs)
Version:
(earliest affected)
7.2.0.0 alpha1+
Hardware: All All
: medium normal
Assignee: Mike Kaganski
URL:
Whiteboard: target:7.6.0 target:7.5.4
Keywords: bibisected, bisected, regression
Depends on:
Blocks: Special-Character
  Show dependency treegraph
 
Reported: 2023-04-18 15:43 UTC by VertD
Modified: 2023-05-02 12:23 UTC (History)
3 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 VertD 2023-04-18 15:43:48 UTC
Description:
After you add an entry to Favorite Special Character list, the button "Add to Favorites" doesn't change to "Remove from Favorites" so its possible to add the same character more than once.

Steps to Reproduce:
1.Go to Insert Special Character
2.Select some char (Latin Capital L) and press button add to favorites.
3.Change Font (Cantarell) and repeat step 2 multiple times.

Actual Results:
You can add the same character more than once.

Expected Results:
The button "Add to Favorites" should change to "Remove from Favorites".

Also try:
1.Go to Insert Special Character
2.Select a Font(Liberation Serif) and some char (Latin Capital L) and press "Add to Favorites".
3.Change Font (Cantarell), select a character (Latin Capital Q) and press "Add to Favorites"(both Q and L are highlighted as favorites).
4.Select L (Button shows "Add to Favorites", context menu shows "Remove from Favorites")
5.Click Remove from Favorites from the context menu.
Actual Results:
The Favorite char Q changes its font to Liberation Serif.
No character is highlighted as favorite.
After step 3 Both Q and L are highlighted as favorites in both Fonts (can't tell if this is expected behavior).
Button and context menu contradicts each other (one shows "Add to Favorites" the other "Remove from Favorites").
Expected Results:
Button and context menu should show the same option.
Char Q shouldn't change its font.(??)
Q should be highlighted as favorite when Font selected is Cantarell and L when Font is Liberation Serif.(??)



Reproducible: Always


User Profile Reset: No

Additional Info:
Version: 7.5.2.2 (X86_64) / LibreOffice Community
Build ID: 50(Build:2)
CPU threads: 4; OS: Linux 6.2; UI render: default; VCL: gtk3
Locale: en-US (en_US.UTF-8); UI: en-US
7.5.2-1
Comment 1 Stéphane Guillou (stragu) 2023-04-19 07:51:33 UTC
Confirmed in recent master build, and bibisected with linux-64-7.2 repo to first bad commit 9db4aa805171cc789d192a1cd339bef76d028a14 which points to core commit:

commit b6ab5914d8b2fc7041430796890f086f8e3e6369
author	Mike Kaganski <mike.kaganski@collabora.com>	Wed Apr 28 12:37:19 2021 +0200
committer	Mike Kaganski <mike.kaganski@collabora.com>	Thu Apr 29 07:17:14 2021 +0200
tdf#135997: make sure that the two lists are same length
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/114722

With steps:

1. Open Special Character dialog, default font is Liberation Serif
2. Select exclamation point, click Add to Favorites
3. Change font to Likhan (or another that also have the exclamation point)
4. Add to Favorite once

Result: button remains active, can be added multiple times.

Mike, can you please have a look?
Comment 2 Mike Kaganski 2023-04-19 08:35:15 UTC
It is the expected thing. You are not adding "abstract character", and when you insert the item from the recents, you are inserting a character with a specific font. The check there in the code does exactly this: checks if the character is there, *and* the font is the same. Consider, for example, some symbol font, or a font with some gothic look: you might want to have the characters from there, independently of their "copies" from other fonts - say, the char from a gothic font could be used in Math, while its copy from symbol font could show some icon in that place...

NAB. I don't mark it as such, up to UX to discuss, and if wanted, to make changes (I can provide code pointers if needed).
Comment 3 Stéphane Guillou (stragu) 2023-04-19 11:52:47 UTC
(In reply to Mike Kaganski from comment #2)
> It is the expected thing. You are not adding "abstract character", and when
> you insert the item from the recents, you are inserting a character with a
> specific font. The check there in the code does exactly this: checks if the
> character is there, *and* the font is the same. Consider, for example, some
> symbol font, or a font with some gothic look: you might want to have the
> characters from there, independently of their "copies" from other fonts -
> say, the char from a gothic font could be used in Math, while its copy from
> symbol font could show some icon in that place...
> 
> NAB. I don't mark it as such, up to UX to discuss, and if wanted, to make
> changes (I can provide code pointers if needed).

Mike, the bug is not that the same character in another font can be also added to the favourite. The bug is that the second character+font combination can be added over and over.
Comment 4 Mike Kaganski 2023-04-24 11:59:58 UTC
(In reply to Stéphane Guillou (stragu) from comment #3)

You are right; I somehow didn't read that in the report. Sorry for that.
Repro.
Comment 5 Mike Kaganski 2023-04-24 12:23:53 UTC
https://gerrit.libreoffice.org/c/core/+/150937
Comment 6 Commit Notification 2023-04-24 14:07:01 UTC
Mike Kaganski committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/5a1d07a819bd0f14dd901b35ae245bcd681cb512

tdf#154884: do not stop if the first pair is not what we looked for

It will be available in 7.6.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 7 Commit Notification 2023-04-25 08:12:53 UTC
Mike Kaganski committed a patch related to this issue.
It has been pushed to "libreoffice-7-5":

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

tdf#154884: do not stop if the first pair is not what we looked for

It will be available in 7.5.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 8 VertD 2023-05-01 00:05:02 UTC
isFavChar was not finding the correct pair char+font, similar with updateFavCharacterList.
This fixes the incorrect highlighting(frame), context menu showing the wrong option and the weird font changing bug.
https://gerrit.libreoffice.org/c/core/+/151216
Comment 9 Stéphane Guillou (stragu) 2023-05-02 11:56:08 UTC
Thanks Mike, I see the main issue resolved in :

Version: 7.6.0.0.alpha0+ (X86_64) / LibreOffice Community
Build ID: fb3a6b82b55a298eabf8f431f1451dc826642acd
CPU threads: 8; OS: Linux 5.15; UI render: default; VCL: gtk3
Locale: en-AU (en_AU.UTF-8); UI: en-US
Calc: threaded

Mike, I added you as a reviewer to Vert's patch. I can still see the context menu vs button mismatch described in "Also try" in Vert's Comment 0.
Comment 10 Commit Notification 2023-05-02 12:23:23 UTC
Vert D committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/332b620732d4f78625c6547a988fa9860ab0d7bd

tdf#154884 fix isFavChar, updateFavCharacterList deletes the correct pair

It will be available in 7.6.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.