Bug 130428 - Try to not use SfxItemState::UNKNOWN in preparation of refactoring of ItemSet/ItemPool
Summary: Try to not use SfxItemState::UNKNOWN in preparation of refactoring of ItemSet...
Status: ASSIGNED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: LibreOffice (show other bugs)
Version:
(earliest affected)
unspecified
Hardware: All All
: medium enhancement
Assignee: Armin Le Grand
URL:
Whiteboard: target:7.0.0 target:7.2.0 target:7.3.0
Keywords:
Depends on:
Blocks:
 
Reported: 2020-02-04 13:26 UTC by Armin Le Grand
Modified: 2021-06-18 09:11 UTC (History)
2 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 Armin Le Grand 2020-02-04 13:26:21 UTC
Background: I do experiment actively with a much simplified, type-safe ersion of ItemSet/ItemPool to make it easier for everyone to deal with these, still keeping performance and mem footprint. Stumbled over SfxItemState::SfxItemState::UNKNOWN which is in the way for that simplifications. Looking for a way around it relealed that it might bne hard and take off quite some of the 'easier' portion, so I'll try to remove it's usages as much as possible.
Im most cases these usages (127) are about checking if that Item is 'known' at the SfxItemSet which uses the info of the defined ranges at these - mostly in UI stuff (anyways a 2nd ItemSet use that is kinda 'hacky') to hide controls.
I will try to replace usages with what really is intended and protocol it here. This might lead to some changes in the core that hopefully will do no harm, but to be able to revert thede and maybe cancel that try here again will not all here
Comment 1 Armin Le Grand 2020-02-04 13:26:50 UTC
Grepping immediately
Comment 2 Armin Le Grand 2020-02-04 15:39:13 UTC
1st commit is: cf4e87469baf13fb2766d0f2593fcc2b9b33bc9b
Comment 3 Commit Notification 2020-02-04 15:39:30 UTC
Armin Le Grand committed a patch related to this issue.
It has been pushed to "master":

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

tdf#130428 SfxItemState::UNKNOWN replacements

It will be available in 7.0.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 4 Commit Notification 2020-02-05 08:10:15 UTC
Julien Nabet committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/9811796aba7360fc5b7230a8b314a56fbf6ab27a

Related tdf#130428: let's add some asserts

It will be available in 7.0.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 Maxim Monastirsky 2020-02-05 21:15:25 UTC
(In reply to Commit Notification from comment #4)
> Julien Nabet committed a patch related to this issue.
> It has been pushed to "master":
> 
> https://git.libreoffice.org/core/commit/
> 9811796aba7360fc5b7230a8b314a56fbf6ab27a
> 
> Related tdf#130428: let's add some asserts
> 
And these asserts can now be triggered in report designer. Just insert a label, and use menu Format > Character...
Comment 6 Armin Le Grand 2020-02-13 08:35:54 UTC
Julien, Maxim: Thanks to you both, adding asserts and getting hins that and HOW these get triggered is what I was hoping for! Will take a 2nd look...
Comment 7 Armin Le Grand 2020-02-13 08:57:00 UTC
> And these asserts can now be triggered in report designer. Just insert a
> label, and use menu Format > Character...

Huh - could not find '...use menu Format > Character...' -> I have no Menu 'Format' and also not in context menu. Can you please describe where exactly you get that menu?
Comment 8 Maxim Monastirsky 2020-02-13 11:41:32 UTC
(In reply to Armin Le Grand from comment #7)
> Huh - could not find '...use menu Format > Character...' -> I have no Menu
> 'Format' and also not in context menu. Can you please describe where exactly
> you get that menu?
Open a database (or create a new one). In the left sidebar switch to "Reports", and click on "Create Report in Design View...". This will open the report builder, which has a Format > Character... menu item.
Comment 9 Armin Le Grand 2020-02-13 14:19:52 UTC
(In reply to Maxim Monastirsky from comment #8)

Thanks, can reproduce now.

Indeed, a AlignmentTabPage is opened, but AlignmentTabPage::GetRanges() is *not* called - sigh.

That whole rptui::openCharDialog constructs it's own ItemPool/ItemSet stuff, including *own* WhichID's starting at XATTR_FILL_LAST. These do not correspond with the WHichID's in cui\source\tabpages\align.cxx *at all*, e.g for the Item SvxHorJustifyItem it uses it's own WhichID ITEMID_HORJUSTIFY which is

XATTR_FILL_LAST + 26 -> 1033 + 26 -> 1059
which gets accessed in the TabPage using SID_ATTR_ALIGN_HOR_JUSTIFY which expands to ( ((10000 + 1499) + 1) + 71 ) -> something completely different. Mapping seems to be done in SfxTabPage::GetWhich which maps to

    sal_uInt16          GetWhich( sal_uInt16 nSlot, bool bDeep = true ) const
                            { return pSet->GetPool()->GetWhich( nSlot, bDeep ); }

SfxItemPool::GetWhich again uses pItemInfos which get set at 4th param in SfxItemPool constructor using aItemInfos which maps { SID_ATTR_ALIGN_HOR_JUSTIFY, true }

So - what does

    sal_uInt16 nWhich = GetWhich(SID_ATTR_ALIGN_HOR_JUSTIFY);

in cui\source\tabpages\align.cxx do...? mnStart/mnEnd are XATTR_FILL_FIRST,ITEMID_WEIGHT_COMPLEX, thus it gets

XATTR_FILL_FIRST + the offset of the found SID_ATTR_ALIGN_HOR_JUSTIFY, so 46.
XATTR_FILL_FIRST is 1014,
XATTR_FILL_LAST is 1033,
#define ITEMID_VERJUSTIFY               XATTR_FILL_LAST + 27

-> indeed leads to the local ITEMID_VERJUSTIFY,l despite being defined based on XATTR_FILL_LAST, not on XATTR_FILL_FIRST.

This is all very tricky and highly depends on these offsets - sigh. That's exactly what I want to get rid of in the ling term to make things easier - but no chance as long as this stuff is mixed as this...

The crash is due to SID_ATTR_ALIGN_STACKED (which is ( SID_SVX_START + 229 )) -> 10229, is indeed *not* defined in aItemInfos for the temporary ItemPool/ItemSet in reportdesign\source\ui\misc\UITools.cxx, thus produces state UNKNOWN.
Comment 10 Armin Le Grand 2020-02-13 14:37:53 UTC
In short: My assumption (hope?) that AlignmentTabPage::GetRange() will always be used is wrong. There are four calls to

    AddTabPage("alignment", RID_SVXPAGE_ALIGNMENT.*

of which three do *not* use a 'TabRangesFunc'-ptr. Thus I have to revert that change and continue thinking about possibilities...
Comment 11 Commit Notification 2020-02-13 21:44:33 UTC
Armin Le Grand (Collabora) committed a patch related to this issue.
It has been pushed to "master":

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

Revert "Related tdf#130428: let's add some asserts"

It will be available in 7.0.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 12 Xisco Faulí 2020-07-22 15:21:01 UTC
Dear Armin Le Grand,
This bug has been in ASSIGNED status for more than 3 months without any
activity. Resetting it to NEW.
Please assign it back to yourself if you're still working on this.
Comment 13 Armin Le Grand 2021-06-09 11:24:09 UTC
Back to assigned...
Comment 14 Armin Le Grand 2021-06-09 11:32:06 UTC
Remove SfxItemState::READONLY now in f3b737ab76efaf1a70dfb22c6b60b08b340cf343 and b60a9bec769339f5f0fe008ef4985263d0cf689e, continuing with checking (slowly) usages of SfxItemState::UNKNOWN
Comment 15 Commit Notification 2021-06-09 15:13:22 UTC
Armin Le Grand (Allotropia) committed a patch related to this issue.
It has been pushed to "master":

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

tdf#130428 remove unnecessary usage of SfxItemState::UNKNOWN

It will be available in 7.2.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 16 Commit Notification 2021-06-10 09:12:10 UTC
Armin Le Grand (Allotropia) committed a patch related to this issue.
It has been pushed to "master":

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

tdf#130428 remove unnecessary usage of SfxItemState::UNKNOWN

It will be available in 7.2.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 17 Commit Notification 2021-06-10 09:12:19 UTC
Armin Le Grand (Allotropia) committed a patch related to this issue.
It has been pushed to "master":

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

tdf#130428 remove unnecessary usage of SfxItemState::UNKNOWN

It will be available in 7.2.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 18 Armin Le Grand 2021-06-10 11:34:05 UTC
After these commits we are down to 105 grep-hits - and the 'simple' cases are mainly done...
Comment 19 Armin Le Grand 2021-06-10 12:28:45 UTC
Back to the basic problem: SfxItemState::UNKNOWN defines a check (see SfxItemSet::GetItemState) if the WhichID is defined in the array of Which Ranges (m_pWhichRanges) of the SfxItemSet.

The GetWhich call e.g. in AlignmentTabPage uses SfxTabPage::GetWhich:

    sal_uInt16          GetWhich( sal_uInt16 nSlot, bool bDeep = true ) const
                            { return pSet->GetPool()->GetWhich( nSlot, bDeep ); }

which lands at SfxItemPool::GetWhich

    sal_uInt16                      GetWhich( sal_uInt16 nSlot, bool bDeep = true ) const;

so this WhichID-replacement usually is bDeep == true which means the check is if in any of the Which Ranges of the hierarchy of SfxItemPools.
This is not very important, we want to get rid of the WhichID-mapping, so do not touch that at all.

More important is the GetItemState call that usually also goes deep:

    SfxItemState                GetItemState(   sal_uInt16 nWhich,
                                                bool bSrchInParent = true,
                                                const SfxPoolItem **ppItem = nullptr ) const;

with bSrchInParent = true being the default.

To replace this in some way we definitely will need a representation of some state like 'explicitely defined for usage in this context' at the SfxItemSet - or better, at it's future replacement.
Comment 20 Armin Le Grand 2021-06-10 13:38:49 UTC
Thus, the Item derivation classes that get used with SfxItemState::UNKNOWN will in some way need to be extra-qualified at the SfxItemSet.

It will not really be possible to find all places where the SfxItemSets for explicitely e.g. the TabPages like AlignmentTabPage will be created - this is also just to unsafe.

For a reliable way to do that we need to identify all usages of the involved WhichIDs and where these get registered at the SfxItemSets, including the nowadays possible merging of those WhichID ranges.

The problem here is to identify all those places, since not really all singe WhichIDs are given, but ranges of those - so no ID to reliably grep for. Maybe some clang extension may help here that can trigger... Have to ask someone who knows better about that than I do.
Comment 21 Armin Le Grand 2021-06-10 15:47:05 UTC
These are the SlotIDs which are checked against SfxItemState::UNKNOWN, I tried to buil Groups (maybe an option to sign functional groups). The ones with TAB in front are double:

Group AlignmentTabPage
SID_ATTR_ALIGN_STACKED
SID_ATTR_ALIGN_ASIANVERTICAL
SID_ATTR_ALIGN_LINEBREAK
SID_ATTR_ALIGN_HYPHENATION
SID_ATTR_ALIGN_SHRINKTOFIT
SID_ATTR_ALIGN_HOR_JUSTIFY
SID_ATTR_ALIGN_INDENT
SID_ATTR_ALIGN_VER_JUSTIFY
SID_ATTR_ALIGN_DEGREES
SID_ATTR_ALIGN_LOCKPOS
SID_ATTR_FRAMEDIRECTION

Group SvxBorderTabPage
SID_ATTR_BORDER_DIAG_TLBR
SID_ATTR_BORDER_DIAG_BLTR
SID_ATTR_ALIGN_MARGIN

Group SvxCharNamePage
SID_ATTR_CHAR_LANGUAGE
SID_ATTR_CHAR_CJK_LANGUAGE
SID_ATTR_CHAR_CTL_LANGUAGE
SID_ATTR_CHAR_COLOR
SID_ATTR_CHAR_WORDLINEMODE
SID_ATTR_CHAR_EMPHASISMARK
SID_ATTR_CHAR_CASEMAP
SID_ATTR_CHAR_RELIEF
SID_ATTR_CHAR_CONTOUR
SID_ATTR_CHAR_SHADOWED
SID_ATTR_CHAR_HIDDEN
SID_ATTR_CHAR_ROTATED
SID_ATTR_CHAR_WIDTH_FIT_TO_LINE

Group SvxPageDescPage
    SID_ATTR_FRAMEDIRECTION
    SID_ATTR_FRAMEDIRECTION

Group SvxExtParagraphTabPage
SID_ATTR_PARA_PAGENUM
SID_ATTR_PARA_FORBIDDEN_RULES
SID_ATTR_PARA_HANGPUNCTUATION
SID_ATTR_PARA_SCRIPTSPACE

Group ScDrawShell
SID_ENABLE_HYPHENATION

Group ScDrawTextObjectBar
SID_HYPERLINK_GETLINK
SID_OPEN_HYPERLINK
SID_EDIT_HYPERLINK
SID_COPY_HYPERLINK_LOCATION
SID_REMOVE_HYPERLINK
SID_TRANSLITERATE_HALFWIDTH
SID_TRANSLITERATE_FULLWIDTH
SID_TRANSLITERATE_HIRAGANA
SID_TRANSLITERATE_KATAKANA
    SID_ENABLE_HYPHENATION
SID_THES
EE_CHAR_FONTINFO
EE_CHAR_FONTHEIGHT
EE_CHAR_WEIGHT
EE_CHAR_ITALIC

ScCellShell
SID_CLIPBOARD_FORMAT_ITEMS

ScEditShell
    EE_CHAR_FONTINFO
    EE_CHAR_FONTHEIGHT
    EE_CHAR_WEIGHT
    EE_CHAR_ITALIC

ScFormatShell
ATTR_FONT
ATTR_FONT_HEIGHT
ATTR_FONT_WEIGHT
ATTR_FONT_POSTURE
ATTR_BORDER
ATTR_BORDER_INNER

SdModule
SID_OPENDOC
SID_OPENHYPERLINK

DrawDocShell
SID_RELOAD

SlideSorterController
    SID_RELOAD

DrawViewShell
    SID_RELOAD

SwGrfExtPage
RES_GRFATR_MIRRORGRF
RES_FRAMEDIR

SwNumPositionTabPage
FN_PARAM_NUM_PRESET

SwTextGridPage
    RES_FRAMEDIR

SfxToSwPageDescAttr
    SID_ATTR_PARA_PAGENUM
Comment 22 Armin Le Grand 2021-06-10 17:29:50 UTC
Found some more doable cases, down to 88 ocurrences
Comment 23 Commit Notification 2021-06-11 07:40:57 UTC
Armin Le Grand (Allotropia) committed a patch related to this issue.
It has been pushed to "master":

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

tdf#130428 remove unnecessary usage of SfxItemState::UNKNOWN

It will be available in 7.2.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 24 Commit Notification 2021-06-11 07:42:09 UTC
Armin Le Grand (Allotropia) committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/552ceeb5c6ccf2573adfd092cc4b2ce214a9c2df

tdf#130428 remove unnecessary usage of SfxItemState::UNKNOWN

It will be available in 7.2.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 25 Commit Notification 2021-06-11 07:42:19 UTC
Armin Le Grand (Allotropia) committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/2baf6f654244a67715e4b3d86c49144ce6b5dd13

tdf#130428 remove unnecessary usage of SfxItemState::UNKNOWN

It will be available in 7.2.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 26 Commit Notification 2021-06-11 14:58:48 UTC
Armin Le Grand (Allotropia) committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/64378ded7b87fa785f13b998e006d87f44a2776d

tdf#130428 remove unnecessary usage of SfxItemState::UNKNOWN

It will be available in 7.2.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 27 Commit Notification 2021-06-11 14:59:59 UTC
Armin Le Grand (Allotropia) committed a patch related to this issue.
It has been pushed to "master":

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

tdf#130428 remove unnecessary usage of SfxItemState::UNKNOWN

It will be available in 7.2.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 28 Commit Notification 2021-06-11 15:00:10 UTC
Armin Le Grand (Allotropia) committed a patch related to this issue.
It has been pushed to "master":

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

tdf#130428 remove unnecessary usage of SfxItemState::UNKNOWN

It will be available in 7.2.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 29 Armin Le Grand 2021-06-11 16:54:15 UTC
SfxItemState::UNKNOWN is also used with controller items, e.g. see SfxControllerItem::GetItemState:

 SfxItemState::UNKNOWN
 Enabled, but no further status information available.
 Typical for <Slot>s, which anyway are sometimes
 disabled, but otherwise do not change their appearance.

This gets used when

 pState->IsVoidItem() && !pState->Which()

which means:

(a) that the SfxPoolItem is a SfxVoidItem (SfxVoidItem::IsVoidItem() const { return true; }), normally used (see SfxItemSet::GetItemState) for SfxItemState::DISABLED

(b) that the 0 == WhichID

That SfxItemState::UNKNOWN is feeded to ::StateChanged calls (52 implementations) for all kinds of controls. Also used in SfxStateCache as local var eLastState to buffer against eState.

Usually this looks like (60 defines in *.hxx)

    // Old sfx2 interface
    virtual void    StateChanged( sal_uInt16 nSID, SfxItemState eState,
                                  const SfxPoolItem* pState );


where

sal_uInt16 nSID -> is the SlotID
SfxItemState eState -> is as described in SfxControllerItem::GetItemState
const SfxPoolItem* pState -> is the SfxPoolItem

For the current SfxItemState::UNKNOWN this gets:

nSID -> exists, identifies a slot
eState -> SfxItemState::UNKNOWN
pState -> SfxVoidItem(0)

Thus there is *no* data from an SfxPoolItem, only the SlotID.

Not sure (yet) how to replace this, but maybe

 SfxItemState::DONTCARE
 Enabled but there were only ambiguous values available
 (i.e. non that can be queried).

would fit well. There cannot really be useful usages in the ::StateChanged methods for this - need to check...

BTW: That description is double, can be found in SfxToolBoxControl::GetItemState and in SfxControllerItem::GetItemState.



AND there is also SfxStatusListener::statusChanged where

            pItem.reset(new SfxVoidItem( m_nSlotID ));
            eState = SfxItemState::UNKNOWN;

is used - in CONTRAST to defines above != SfxVoidItem(0), but also feed to a

    StateChanged( eState, pItem.get() );

that does *not* take a SlotID -> maybe another hack which needs to be solved...?
Both ::GetItemState mentioned above will in tis case return SfxItemState::DEFAULT instead of SfxItemState::UNKNOWN -> is that WANTED?
Comment 30 Armin Le Grand 2021-06-17 07:40:18 UTC
Checked for StateChanged:
(1) SfxToolBoxControl:
    virtual void               StateChanged( sal_uInt16 nSID, SfxItemState eState, const SfxPoolItem* pState );
(2) SfxStatusListener:
        virtual void StateChanged( SfxItemState eState, const SfxPoolItem* pState );
(3) SfxStatusBarControl:
    virtual void    StateChanged( sal_uInt16 nSID, SfxItemState eState,
                                  const SfxPoolItem* pState );
(4) Control, vcl::Window:
    virtual void                        StateChanged( StateChangedType nStateChange );

(1,2,3) are used for StateChange with SfxItemState parameter. To be able to better check/read/analyze these I propose a NameChange to allow direct grepping/searching.

(1,3) even have same parameter signature, (2) avoids this by 'handing' over nSID inside the SfxItem (if available - as mentioned in comment 29 this leads to use 'hacks' to hand in a SfxVoidItem(nSID) as parameter transport

(4) s not interesting in this case - just vcl::Window handling using StateChangedType, no coincidence with SfxItemState at all.

-> Rename (1,2,3)
-> Keep (4), do not touch
-> for now, ignore hacks at (2), do not extend parameters/signature
Comment 31 Commit Notification 2021-06-18 09:11:49 UTC
Armin Le Grand (Allotropia) committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/04cd6749177f886f382e8bcd026f95112ee22473

tdf#130428 remove unnecessary usage of SfxItemState::UNKNOWN

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