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.alpha1+
Hardware: All All
: medium normal
Assignee: Justin L
URL:
Whiteboard: target:7.1.0 target:7.0.0.1 target:6.4.5
Keywords: bibisected, bisected, regression
Depends on: 125677
Blocks: Form-Controls
  Show dependency treegraph
 
Reported: 2019-05-31 12:34 UTC by Oliver Brinzing
Modified: 2020-06-04 04:58 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 Stéphane Guillou (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.
Comment 20 Justin L 2019-11-14 16:56:37 UTC
The fixes made in this bug report are not strictly needed any more, since the original regression commit has been fully reverted. But they all seemed to be legitimate fixes in their own right, so leaving them alone as long as they don't cause any trouble.
Comment 21 Justin L 2020-05-07 05:13:56 UTC
(In reply to Justin L from comment #14)
> Comment 13's master and 6.3 patch
has been reverted in LO 6.4 to resolve bug 132581. It is not being reverted for master, so it will still be applied to 7.0.
Comment 22 Commit Notification 2020-06-01 16:42:53 UTC
Noel Grandin committed a patch related to this issue.
It has been pushed to "master":

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

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

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 23 Commit Notification 2020-06-01 17:40:49 UTC
Justin Luth committed a patch related to this issue.
It has been pushed to "master":

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

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

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 24 Commit Notification 2020-06-02 16:38:42 UTC
Noel Grandin committed a patch related to this issue.
It has been pushed to "libreoffice-7-0":

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

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

It will be available in 7.0.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 25 Commit Notification 2020-06-03 07:36:16 UTC
Noel Grandin committed a patch related to this issue.
It has been pushed to "libreoffice-6-4":

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

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

It will be available in 6.4.5.

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 2020-06-04 04:19:47 UTC
Justin Luth committed a patch related to this issue.
It has been pushed to "libreoffice-7-0":

https://git.libreoffice.org/core/commit/766be0a8f2f5abec29709ef7317d8a0140fda70d

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

It will be available in 7.0.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 27 Justin L 2020-06-04 04:58:49 UTC
Summary of the convoluted changes here:
-this bug reports problem caused by patch
    tdf#108687 vcl: always enable tabstop on radio buttons
was eliminated by completely reverting it from everywhere. So this bug remains "fixed" since the cause of the problem was removed.


And now, all the patches attempting to fix the fixes have also been removed. So in the end result, approximately nothing has changed.
-comment 13's patch has been fully reverted to 6.4.5
-the accompanying comment 18 patch has been reverted to 7.0, but ends up just being unused bits in 6.4 and 6.3.
-comment 15's patch has been fully reverted to 6.4.5.