Bug 135178 - Styles and Formatting tree not updated
Summary: Styles and Formatting tree not updated
Status: RESOLVED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: Writer (show other bugs)
Version:
(earliest affected)
7.1.0.0.alpha0+
Hardware: All All
: medium normal
Assignee: Shivam Kumar Singh
URL:
Whiteboard: target:7.1.0
Keywords:
: 135182 (view as bug list)
Depends on:
Blocks: Style-Inspector
  Show dependency treegraph
 
Reported: 2020-07-27 08:34 UTC by Heiko Tietze
Modified: 2020-07-30 07:45 UTC (History)
5 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 Heiko Tietze 2020-07-27 08:34:07 UTC
In a text with different paragraph styles it's expected that on selection the S&F tree changes accordingly. For example you click on a heading and the "Heading 1" node is highlighted, click on some text and "Text Body" is highlighted.

This functionality is broken with the introduction of the Styles Inspector, bug 134554.
Comment 1 Mike Kaganski 2020-07-27 08:41:55 UTC
Caused by grabbing the change link exclusively by SI in WriterInspectorTextPanel::WriterInspectorTextPanel (calling SwWrtShell::SetChgLnk). We must either make it chained (e.g. store previously set link in a member variable, and call it before/after we execute code in our link), or modify SI to not set the link, but to use the existing link (investigate how sidebar/toolbar get notifications and don't step on each other's toes).
Comment 2 Mike Kaganski 2020-07-27 10:37:29 UTC
(In reply to Mike Kaganski from comment #1)
> ..., or modify SI to not set the link, but to use the
> existing link (investigate how sidebar/toolbar get notifications and don't
> step on each other's toes).

See SwXTextView::addSelectionChangeListener. This would be the preferred way to solve this.
Comment 3 Heiko Tietze 2020-07-27 12:31:06 UTC
*** Bug 135182 has been marked as a duplicate of this bug. ***
Comment 4 Commit Notification 2020-07-28 19:19:50 UTC
Shivam Kumar Singh committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/3b8c893b487a3ee27bac710da74856225fb72950

tdf#135178 tdf#135179 tdf#134820 Issue in SetChgLnk in Inspector

It will be available in 7.1.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 5 Mike Kaganski 2020-07-28 21:09:28 UTC
Thank you Shivam for fixing this!

In the meanwhile, I have checked how style is updated in Stylist; looks like it listens to SfxHintId::UpdateDone. Which at some point we also need to do; that would allow the update be asynchronous, thus avoiding performance issues, and reverting the fix to avoid using change link at all.

Possibly we would simply need to inherit the inspector implementation from SfxCommonTemplateDialog_Impl, which handles the notification?
Comment 6 Mike Kaganski 2020-07-28 21:16:29 UTC
... or modify IMPL_LINK_NOARG(SwView, AttrChangedNotify, LinkParamNone*, void) to send an asynchronous notification
Comment 7 Shivam Kumar Singh 2020-07-30 07:27:26 UTC
(In reply to Mike Kaganski from comment #5)
 
> Possibly we would simply need to inherit the inspector implementation from
> SfxCommonTemplateDialog_Impl, which handles the notification?

Inspector implementation from SfxCommonTemplateDialog_Impl ? Could you please explain a bit on this Mike.

Thanks
Comment 8 Mike Kaganski 2020-07-30 07:45:14 UTC
(In reply to Shivam Kumar Singh from comment #7)

SfxCommonTemplateDialog_Impl::Notify gets notified on SfxHintId::UpdateDone, and I thought we could expand that to call some virtual function ... but it seems that it doesn't react for some cases we need, so likely comment 6 is more relevant - please ignore "inherit from SfxCommonTemplateDialog_Impl" in comment 5.

The thing is, currently we call the update code for each invocation of change link, which happens synchronously. The intended end result would be to refresh asynchronously: the pre-existing change link would fire an idle timer, and it would only execute once after LO finished executing more prioritized tasks, so that SI doesn't incur performance penalty. But that's for later.