Comment 1 Pierre-Charles David 2018-09-21 11:26:40 EDT

In `ModelExplorerView` we do:

    @Override
    public void createPartControl(Composite aParent) {
        aParent.setLayout(new FillLayout(SWT.HORIZONTAL));

i.e. we confgure the "aParent" composite to use a FillLayout to layout its children. After a few more calls...

oes.ui.tools.internal.views.common.navigator.filter.FilteredCommonTree.<init>(java.lang.String, org.eclipse.swt.widgets.Composite, int, boolean) line: 228	
oes.ui.tools.internal.views.modelexplorer.ModelExplorerView.createCommonViewerObject(org.eclipse.swt.widgets.Composite) line: 321	
oes.ui.tools.internal.views.modelexplorer.ModelExplorerView(org.eclipse.ui.navigator.CommonNavigator).createCommonViewer(org.eclipse.swt.widgets.Composite) line: 495	
oes.ui.tools.internal.views.modelexplorer.ModelExplorerView(org.eclipse.ui.navigator.CommonNavigator).createPartControl(org.eclipse.swt.widgets.Composite) line: 187	
oes.ui.tools.internal.views.modelexplorer.ModelExplorerView.createPartControl(org.eclipse.swt.widgets.Composite) line: 102	

we end up creating a FilteredCommonTree with this composite as parent, still configured to use a FillLayout. Then in FilteredCommonTree.createControl(Composite, int) we do:

    protected void createControl(Composite aParent, int treeStyle) {
        GridLayout layout = new GridLayout();
        layout.marginHeight = 0;
        layout.marginWidth = 0;
        setLayout(layout);
        setLayoutData(new GridData(SWT.FILL, SWT.FILL, true, true));

I'm not sure what was the original intent, but we associate an a GridData to the FilteredCommonTree although its parent (the composite) is configured with a FillLayout.

This fails a little later with

java.lang.ClassCastException: org.eclipse.swt.layout.GridData cannot be cast to org.eclipse.swt.layout.FillData
	at org.eclipse.swt.layout.FillLayout.flushCache(FillLayout.java:176)
	at org.eclipse.swt.widgets.Composite.layout(Composite.java:834)
	at org.eclipse.swt.widgets.Control.requestLayout(Control.java:2414)
	at org.eclipse.jface.action.ToolBarManager.relayout(ToolBarManager.java:224)
	at org.eclipse.jface.action.ToolBarManager.update(ToolBarManager.java:384)

when the FillLayout is called and assumes all the children to layout are correctly setup with a FillData.

Comment 2 Pierre-Charles David 2018-09-21 11:34:15 EDT

The bug only happens under Windows as it is triggered by filterToolBar.update(false) in FilteredCommonTree.createFilterControls(Composite), but this call is never made under Linux because here filterToolBar is never initialized. In FilteredCommonTree.createClearTextOld(Composite), filterToolBar is created only if the text field doesn't have the SWT.ICON_CANCEL flag, and this is only set depending on the result of FilteredCommonTree.useNativeSearchField(aParent), which is OS-dependant.

Up to (and including) Photon, the inconsistancy was present but for some reason never triggered a failure. The change made in ToolBarManager.relayout(ToolBar, int, int) by commit https://git.eclipse.org/r/c/125161 changed that, and starting with JFace 3.14.100 (first available in SimRel 2018-09), we now get a ClassCastException and a complete failure to initialize thee Model Explorer view.

Comment 3 Pierre-Charles David 2018-09-21 11:35:22 EDT

Created attachment 275933 [details]
The Model Explorer view failing to initialize

Comment 4 Pierre-Charles David 2018-09-24 05:53:50 EDT

Sirius's FilteredCommonTree class was initially (a looong time ago) a copy of JFace's org.eclipse.ui.dialogs.FilteredTree. The bug was there at the time in the original. It was fixed in FilteredTree by commit:

  c9e5e2d2d0c28bcc0fef48dcca2be87cc429db51 "Fixed bug 226155: FilteredTree assumes parent uses GridLayout"

See https://git.eclipse.org/r/plugins/gitiles/platform/eclipse.platform.ui/+/c9e5e2d2d0c28bcc0fef48dcca2be87cc429db51%5E%21/#F0

Of course we did not notice that, and our own version has been exposed to the bug until now.

The fix will be simply to apply to our version the same change that was made in FilteredTree. In addition I'll try to have a look at *why* we needed (or thought we needed) to create a whole copy of the JFace class, instead of subclassing it (for example).

Comment 5 Eclipse Genie 2018-09-24 08:45:04 EDT

New Gerrit change created: https://git.eclipse.org/r/129866

Comment 6 Eclipse Genie 2018-09-24 09:38:54 EDT

Gerrit change https://git.eclipse.org/r/129866 was merged to [v6.0.x].
Commit: http://git.eclipse.org/c/sirius/org.eclipse.sirius.git/commit/?id=ad152bfcec01d6154d037808e6de85409da82c2c

Comment 7 Pierre-Charles David 2018-09-24 10:12:30 EDT

Fixed by ad152bfcec01d6154d037808e6de85409da82c2c.

There are no automated tests:
* our test suites are normally run under Linux, so wouldn't show the problem;
* under Windows, almost every single (SWTbot) test which tries to interact with the Model Explorer view would already fail with this bug, as long as we are in the correct context (Windows, SimRel 2018-09 or later).

Steps to validate, under Windows:
1. Install an Eclipse Commiters package (any package without Sirius pre-installed, so any except Modeling) from SimRel 2018-09 (or later).
2. Launch Eclipse and install the Sirius version begin validated from its repository, e.g. http://download.eclipse.org/sirius/updates/nightly/6.0.x/photon
3. Restart; switch the the "Modeling" perspective: the "Model Explorer" view in the top-left should be initialized without error.

Comment 8 Jessy Mallet 2018-09-25 09:25:23 EDT

Validated on Sirius 6.0.2 (6.0.2.201809241340)

Comment 9 Eclipse Genie 2018-09-25 09:36:19 EDT

New Gerrit change created: https://git.eclipse.org/r/129934

Comment 10 Eclipse Genie 2018-09-25 10:54:26 EDT

Gerrit change https://git.eclipse.org/r/129934 was merged to [v6.0.x].
Commit: http://git.eclipse.org/c/sirius/org.eclipse.sirius.git/commit/?id=15b4d163768790378157f76dc6b96cb2742ee464

Comment 11 Pierre-Charles David 2018-09-25 12:21:14 EDT

Available in Sirius 6.0.2: https://wiki.eclipse.org/Sirius/6.0.2

Comment 12 Pierre-Charles David 2018-10-11 08:36:10 EDT

*** Bug 540039 has been marked as a duplicate of this bug. ***