Bug Hunting Session
Bug 125609 - radiobutton receives item status changed event after listbox value selection
Summary: radiobutton receives item status changed event after listbox value selection
Status: RESOLVED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: framework (show other bugs)
Version:
(earliest affected)
6.4.0.0.alpha0+ Master
Hardware: All All
: medium normal
Assignee: Justin L
URL:
Whiteboard: target:6.4.
Keywords: bibisected, bisected, regression
Depends on: 125677
Blocks: Form-Controls
  Show dependency treegraph
 
Reported: 2019-05-31 12:34 UTC by Oliver Brinzing
Modified: 2019-08-01 18:38 UTC (History)
4 users (show)

See Also:
Crash report or crash signature:


Attachments
test_itemStatusChangedEvent_tabstop_yes_basic_simple (11.98 KB, application/vnd.oasis.opendocument.spreadsheet)
2019-05-31 12:35 UTC, Oliver Brinzing
Details
test_itemStatusChangedEvent_tabstop_no_basic_simple_lo61 (12.58 KB, application/vnd.oasis.opendocument.spreadsheet)
2019-05-31 12:47 UTC, Oliver Brinzing
Details
patch125609.diff: debug and almost-fix (3.12 KB, patch)
2019-06-06 12:15 UTC, Justin L
Details
tdf108687_tabstop.odt: slightly different - from writer instead of calc. Test fixes against this too. (12.55 KB, application/vnd.oasis.opendocument.text)
2019-06-06 12:23 UTC, Justin L
Details
multipleRadioButtonsChecked-grammalecte.oxt: minimal example with all the radio buttons checked (94.06 KB, application/vnd.openofficeorg.extension)
2019-06-19 05:57 UTC, Justin L
Details
test_itemStatusChangedEvent_tabstop_no_basic_simple_lo61_noMacro.ods: crash sample (10.92 KB, application/vnd.oasis.opendocument.spreadsheet)
2019-07-23 12:54 UTC, Justin L
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Oliver Brinzing 2019-05-31 12:34:32 UTC
if a listbox is connected to a macro via item status changed event and the macro disables the listbox control while the code is running after a selection was made, a second event is fired for a radiobutton.

steps to reproduce:
- open attached spreadsheet
- select a value from listbox, e.g. "9"
-> MsgBox "RadioButton 2" appears

not reproducible with LO 6.1.6.3

i was able to find the problematic commit:

https://gerrit.libreoffice.org/plugins/gitiles/core/+/f2cd1c3c7cce2699d1341f726fc90cf30b52612c

gerrit.libreoffice.org / core / f2cd1c3c7cce2699d1341f726fc90cf30b52612c
commit	f2cd1c3c7cce2699d1341f726fc90cf30b52612c	[log]
author	Justin Luth <justin.luth@collabora.com>	Sat Nov 03 21:56:40 2018 +0300
committer	Caolán McNamara <caolanm@redhat.com>	Mon Nov 05 10:02:12 2018 +0100
tree	15cbd44f997070f3218f250df0f3e475b957d99a
parent	cb9d192f18082d517b805f5d84fc62bde6319ded [diff]

tdf#108687 vcl: always enable tabstop on radio buttons

...as long as they are not WB_NOTABSTOP of course,
just like checkboxes.

Even though all of the radio buttons are now marked
as WB_TABSTOP, the tab still only stops once inside
the radio group. That's perfect.

Without this, if none of the radio boxes was checked,
then the entire group was tab-skipped.

As a copy/paste programmer, I don't know how to
test this because there were no close enough examples
to follow.

Change-Id: I3c559fb274d736cbd2f56a6a8ddc1ca5a2cfe681
Reviewed-on: https://gerrit.libreoffice.org/62822
Tested-by: Jenkins
Reviewed-by: Caolán McNamara <caolanm@redhat.com>
Tested-by: Caolán McNamara <caolanm@redhat.com>
Comment 1 Oliver Brinzing 2019-05-31 12:35:43 UTC
Created attachment 151805 [details]
test_itemStatusChangedEvent_tabstop_yes_basic_simple
Comment 2 Oliver Brinzing 2019-05-31 12:47:06 UTC
Created attachment 151807 [details]
test_itemStatusChangedEvent_tabstop_no_basic_simple_lo61

the same problem is already reproducible with LO 6.1, if the radio buttons "Tabstop" attributes are set to "No"
Comment 3 stragu 2019-05-31 13:18:02 UTC
Although I am not entirely sure I understand what the issue is, I can confirm the described behaviour on:

Version: 6.2.4.2
Build ID: 1:6.2.4-0ubuntu0.18.04.1~lo1
CPU threads: 8; OS: Linux 4.15; UI render: default; VCL: gtk3; 
Locale: en-AU (en_AU.UTF-8); UI-Language: en-GB
Calc: threaded
Comment 4 Justin L 2019-06-03 19:12:57 UTC
confirmed. When the document starts, note that RadioButton1 is selected. If the user does not touch the radio buttons, then when the combo box disables itself, control switches over to the radio buttons - and it switches over to radiobutton2 - thus the event.

If you first "confirm" that radiobutton1 is selected, then the problem is not seen.
Comment 5 Justin L 2019-06-06 12:15:42 UTC
Created attachment 151964 [details]
patch125609.diff: debug and almost-fix

Test the following situations:
-only one radio-button form control - no infinite loops
-a checkbox and only one radio-button - again no infinite loops
-test against uiwriter/data2/tdf108687_tabstop.odt - multiple groups

The debug indicates that there are NO identified groups in OP's example document (which is incorrect of course. The buttons have the identical name, which is the old-school way of grouping radio buttons). So that suggests there are more things messed up here. If the groups where OK, then this patch should work. (However, there are still other things at play, since in LO 6.2, just verifying that radio-button1 is selected is enough to "fix" the problem. I think that the act of selecting it removes the tabstop from all others. How it knows which radio-buttons to de-tabstop is still a mystery to me.)
Comment 6 Justin L 2019-06-06 12:23:24 UTC
Created attachment 151965 [details]
tdf108687_tabstop.odt: slightly different - from writer instead of calc. Test fixes against this too.

(In reply to Justin L from comment #5)
>missing groupings / other things at play / mysteries
Given all this, I think I'll just revert my patch in the 6.2 branch so that it remains effectively untouched. Then we can try to sort out the multiple regressions occurring in 6.3.
Comment 7 Justin L 2019-06-07 11:15:11 UTC
(In reply to Justin L from comment #5)
> How it knows which radio-buttons to de-tabstop is still a mystery to me.
Ends up being from forms ORadioButtonModel::SetSiblingPropsTo, when it calls vcl RadioButton::Check(false).
Comment 8 Justin L 2019-06-08 17:57:57 UTC
This is where I am getting stuck. Every time a control is added, it inefficiently re-creates everything, except it doesn't run after the last control is added.

toolkit/source/controls/stdtabcontroller.cxx:79: ::ImplCreateComponentSequence model[0x55e5040e19d8][2}of[3] FindControl for it. Controls[2]
toolkit/source/controls/stdtabcontroller.cxx:81: ---found?[0] realControls up till now[2]
toolkit/source/controls/stdtabcontroller.cxx:362: ::activateTabOrder for group[0] of[1] name[G_Optionsfeld]
toolkit/source/awt/vclxcontainer.cxx:223: GDB grab point - last item in the group - looking for behindlast finds the radiobutton that has no control yet
toolkit/source/awt/vclxcontainer.cxx:225: xxx me[0x55e5044f2f80] next window[0x55e50466b440] are the same?[0]

So here, it found controls for two of the three models. Apparently the second radiobutton hasn't been created with a control yet, so it sees it as a BehindLast window, and marks it as a new group. Any future attempt to activateTabOrder fail without a peerContainer (after a resumeTabOrderUpdate).
toolkit/source/controls/stdtabcontroller.cxx:321: ::activateTabOrder controlcontainer.is[1] peerContainer.is[0].
Comment 9 Justin L 2019-06-10 19:32:23 UTC
(In reply to Justin L from comment #6)
> I'll just revert my patch in the 6.2 branch so that
> it remains effectively untouched. Then we can try to sort out the multiple
> regressions occurring in 6.3.
Completed for 6.2.5. So this is now just a LO 6.3 bug.
Comment 10 Olivier R. 2019-06-11 13:35:04 UTC
I was about to create a new bug when I discovered this one.
The bug I’ve encountered looks different from the one reported here, even if it is related to RadioButton also. Though when I tested the last build of LO 6.2.5, it was solved. So I assume it is somehow related.

So I report the issue I have seen, for informative purpose, in case it might help. Sorry if it doesn’t.

Bug from LO 6.2.0 to LO 6.2.4.

If I build a dialog box with RadioBoxes, then update the State of a RadioBox, the change is ignored.
The RadioButton is not updated.

Here is the code :

class MyDialogBox (unohelper.Base, XActionListener, XJobExecutor):

    def __init__ (self, ctx):
        self.ctx = ctx
        self.xSvMgr = self.ctx.ServiceManager
        self.xContainer = None
        self.xDialog = None

    def _addWidget (self, name, wtype, x, y, w, h, **kwargs):
        xWidget = self.xDialog.createInstance('com.sun.star.awt.UnoControl%sModel' % wtype)
        xWidget.Name = name
        xWidget.PositionX = x
        xWidget.PositionY = y
        xWidget.Width = w
        xWidget.Height = h
        for k, w in kwargs.items():
            setattr(xWidget, k, w)
        self.xDialog.insertByName(name, xWidget)
        return xWidget

    def run (self, sLang):
        self.xOptionNode = helpers.getConfigSetting("/org.openoffice.MyAddonNode/Options/", True)

        # dialog
        self.xDialog = self.xSvMgr.createInstanceWithContext('com.sun.star.awt.UnoControlDialogModel', self.ctx)
        ...
        self.xRadioButton1 = self._addWidget('radio1', 'RadioButton', ...)
        self.xRadioButton2 = self._addWidget('radio2', 'RadioButton', ...)
        self.xRadioButton3 = self._addWidget('radio3', 'RadioButton', ...)
        ...

        # reading options from configuration
        xChild = self.xOptionNode.getByName("option_node")
        sValue = xChild.getPropertyValue("option_value")
        if sValue == "something":
            self.xRadioButton1.State = 1
        elif sValue = "another_thing":
            self.xRadioButton2.State = 1
        else:
            self.xRadioButton3.State = 1
        # !!!!!!!!!!!!!!!!!!!!!! BUG HERE !!!!!!!!!!!!!!!!!!!!!!!!!
        # no change visible on the dialog box
        # the first radio box is selected whatever the value is.

        # container
        self.xContainer = self.xSvMgr.createInstanceWithContext('com.sun.star.awt.UnoControlDialog', self.ctx)
        self.xContainer.setModel(self.xDialog)
        ...
        self.xContainer.setVisible(False)
        toolkit = self.xSvMgr.createInstanceWithContext('com.sun.star.awt.ExtToolkit', self.ctx)
        self.xContainer.createPeer(toolkit, None)
        self.xContainer.execute()
Comment 11 Justin L 2019-06-12 08:37:00 UTC
(In reply to Olivier R. from comment #10)
Olivier is talking about the Grammalecte extension. particularly the menu Grammalecte -> Graphics Options page, where the default radio button should be the second one (thick wiggly line). Instead, the first one (thin wiggly line) is selected. https://extensions.libreoffice.org/extensions/grammalecte

Bisected to confirm that it was the same commit.
Comment 12 Justin L 2019-06-19 05:57:00 UTC
Created attachment 152284 [details]
multipleRadioButtonsChecked-grammalecte.oxt: minimal example with all the radio buttons checked

proposed fix for comments 10/11 at https://gerrit.libreoffice.org/74108
    tdf#125609c10 vcl/button: enforce only one radio selected on init

which will also solve the problem of ignoring the "checked" status if the first radio is marked as tabstop - which was the case in the Grammalecte extension.
Comment 13 Commit Notification 2019-06-28 07:52:30 UTC
Justin Luth committed a patch related to this issue.
It has been pushed to "libreoffice-6-3":

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

tdf#125609 c10 vcl/button: enforce only one radio selected on init

It will be available in 6.3.0.1.

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 14 Justin L 2019-06-28 08:26:32 UTC
Comment 13's master and 6.3 patch does not solve the main LO 6.2 problem of comment 0, but only fixes the related problem from comments 10/11. Leaving the issue open.
Comment 15 Commit Notification 2019-07-19 04:15:22 UTC
Justin Luth committed a patch related to this issue.
It has been pushed to "master":

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

tdf#125609 toolkit: don't use XTabController::getControls

It will be available in 6.4.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 Justin L 2019-07-19 05:11:22 UTC
(In reply to Justin L from comment #6)
> I'll just revert my patch in the 6.2 branch
Also reverted for 6.3.0. So this is now just a LO 6.4 bug which should be fixed by comment 15's commit. However I just noticed that it seems to have exposed a situation that crashes the example from comment 2 with an infinite loop of state changes. Arggh.
Comment 17 Justin L 2019-07-23 12:54:28 UTC
Created attachment 152960 [details]
test_itemStatusChangedEvent_tabstop_no_basic_simple_lo61_noMacro.ods: crash sample

(In reply to Justin L from comment #13)
This commit could cause a crash loop, so a proposed patch for that is at /gerrit.libreoffice.org/76186

By attaching the example document to bugzilla, the document will eventually find it's way as a sample file in the crash-testing unit tests. That is much nicer than actually adding it to the sc/qa tests which everyone runs...
Comment 18 Commit Notification 2019-07-30 05:18:40 UTC
Justin Luth committed a patch related to this issue.
It has been pushed to "master":

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

tdf#125609 c14: vcl button: don't modify style directly

It will be available in 6.4.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 19 Commit Notification 2019-07-30 07:36:12 UTC
Justin Luth committed a patch related to this issue.
It has been pushed to "libreoffice-6-3":

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

tdf#125609 c14: vcl button: don't modify style directly

It will be available in 6.3.1.

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.