Bug 574819 - Views have no close button in workspace created with Eclipse older 4.8 M2
Summary: Views have no close button in workspace created with Eclipse older 4.8 M2
Status: NEW
Alias: None
Product: Platform
Classification: Eclipse Project
Component: UI (show other bugs)
Version: 4.17   Edit
Hardware: PC Linux
: P3 normal (vote)
Target Milestone: ---   Edit
Assignee: Platform-UI-Inbox CLA
QA Contact:
URL:
Whiteboard:
Keywords:
: 567054 574836 575128 (view as bug list)
Depends on:
Blocks:
 
Reported: 2021-07-13 07:18 EDT by Simeon Andreev CLA
Modified: 2021-08-25 13:07 EDT (History)
6 users (show)

See Also:


Attachments
Screenshot showing the missing close buttons on views. (49.92 KB, image/png)
2021-07-13 07:18 EDT, Simeon Andreev CLA
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Simeon Andreev CLA 2021-07-13 07:18:56 EDT
Created attachment 286777 [details]
Screenshot showing the missing close buttons on views.

To reproduce:

1. Create a workspace with Eclipse 4.8 M1 (20170802-2000), close the welcome page and close Eclipse.
2. Start Eclipse 4.17+ (20200705-1800 and later) in the workspace.
3. Observe that active views have no close button.

We assume this is due to differences in workbench.xmi, some element being handled differently.

When the workspace is created with Eclipse 4.8 M2 (20170913-2000), the problem is not seen. Also when starting Eclipse 4.17 20200629-1800 and earlier, in the Eclipse 4.8 M1 workspace, the problem is not seen.

Unfortunately I have no other Eclipse versions available to make the intervals tighter.

The relevant differences in workbench.xmi between Eclipse 4.8 M1 and 4.8 M2 seem to be of this type:

-              <children xsi:type="advanced:Placeholder" xmi:id="_ERSZtuPEEeuw3pTLcdSoRg" elementId="org.eclipse.jdt.junit.ResultView" toBeRendered="false" ref="_ERRLkOPEEeuw3pTLcdSoRg"/>
+              <children xsi:type="advanced:Placeholder" xmi:id="_D_e4A-PEEeuRNoCjWXGypA" elementId="org.eclipse.jdt.ui.PackageExplorer" ref="_D_T44OPEEeuRNoCjWXGypA" closeable="true">
+                <tags>View</tags>
+                <tags>categoryTag:Java</tags>
+              </children>

So when the new tags are missing, Eclipse 4.17 20200705-1800 and later runs into the problem.
Comment 1 Andrey Loskutov CLA 2021-07-13 07:39:13 EDT
@Simeon: does this only affect 4.8 M1, or a regular 4.7 and earlier releases too?

@Rolf, this seem to be similar to bug 574805, any ideas what can go wrong here? Do we expect some dedicated tags now in the model?
Comment 2 Simeon Andreev CLA 2021-07-13 07:44:35 EDT
(In reply to Andrey Loskutov from comment #1)
> @Simeon: does this only affect 4.8 M1, or a regular 4.7 and earlier releases
> too?

I see the problem with workspaces created by the following builds:

* 4.7 20170612-0950
* 4.8 M1, 20170802-2000
* 4.7.2 20171130-0510
* 4.7.3 20180213-0600

I assume earlier versions (e.g. 4.6 and below) also cause the problem.

The first build that I have (and I've tried) that doesn't create a workspace causing the problem is: 4.8 M2, 20170913-2000
Comment 3 Rolf Theunissen CLA 2021-07-13 09:22:46 EDT
In Bug 433465 the StackRenderer#isCloseable is changed to take the 'closeable' attribute into account for Placeholders. In the xmi snippet it shows that this attribute is not stored before 4.18M2, so it has the default value 'false'.

The tags and closable state are copied from the Part to the Placeholder as of Bug 516403 in PartServiceImpl#createSharedPart(MPart sharedPart). 

Bug 433465 causes this regression if the workbench model is created before Bug 516403.
Comment 4 Andrey Loskutov CLA 2021-07-13 09:30:45 EDT
(In reply to Rolf Theunissen from comment #3)
> In Bug 433465 the StackRenderer#isCloseable is changed to take the
> 'closeable' attribute into account for Placeholders. In the xmi snippet it
> shows that this attribute is not stored before 4.18M2, so it has the default
> value 'false'.
> 
> The tags and closable state are copied from the Part to the Placeholder as
> of Bug 516403 in PartServiceImpl#createSharedPart(MPart sharedPart). 
> 
> Bug 433465 causes this regression if the workbench model is created before
> Bug 516403.

Thanks Rolf.

So in isClosable() 

https://git.eclipse.org/r/c/platform/eclipse.platform.ui/+/163756/4/bundles/org.eclipse.e4.ui.workbench.renderers.swt/src/org/eclipse/e4/ui/workbench/renderers/swt/StackRenderer.java

if we have *no* tags, can we assume the view is closeable?
Comment 5 Simeon Andreev CLA 2021-07-13 09:38:37 EDT
(In reply to Andrey Loskutov from comment #4)
> So in isClosable() 
> 
> https://git.eclipse.org/r/c/platform/eclipse.platform.ui/+/163756/4/bundles/
> org.eclipse.e4.ui.workbench.renderers.swt/src/org/eclipse/e4/ui/workbench/
> renderers/swt/StackRenderer.java

I can confirm, reverting this change "fixes" the problem.
Comment 6 Christoph Laeubrich CLA 2021-07-13 09:50:41 EDT
Won't it help to reset / clear the persisted state then?
Comment 7 Rolf Theunissen CLA 2021-07-13 09:52:38 EDT
(In reply to Andrey Loskutov from comment #4)
> So in isClosable() 
> 
> https://git.eclipse.org/r/c/platform/eclipse.platform.ui/+/163756/4/bundles/
> org.eclipse.e4.ui.workbench.renderers.swt/src/org/eclipse/e4/ui/workbench/
> renderers/swt/StackRenderer.java
> 
> if we have *no* tags, can we assume the view is closeable?

No, we cannot, in fact that is bug 433465. The closeable state of a placeholder could depended on:
1) the closable attribute of the placeholder (isClosable() method)
2) the IPresentationEngine.NO_CLOSE tag of the placeholder
3) the closable attribute of the referenced part
4) the IPresentationEngine.NO_CLOSE tag of the referenced part

The current implementation takes 1 + 2 into account for placeholders. Which makes sense to me. For parts the IPresentationEngine.NO_CLOSE is totally ignored.

In fact, IPresentationEngine.NO_CLOSE could be replaced by using the close attribute only.
Comment 8 Rolf Theunissen CLA 2021-07-13 09:56:02 EDT
(In reply to Christoph Laeubrich from comment #6)
> Won't it help to reset / clear the persisted state then?

That would help, at the expense of loosing customization in the workbench layout.

Moreover, see the short discussion in Bug 566074, how old workbenches should be supported in automatic migrations?
Comment 9 Andrey Loskutov CLA 2021-07-13 09:56:57 EDT
(In reply to Christoph Laeubrich from comment #6)
> Won't it help to reset / clear the persisted state then?

You mean, deleting workbench.xmi? How an average user should know that :-) ?
Comment 10 Rolf Theunissen CLA 2021-07-13 09:57:57 EDT
(In reply to Rolf Theunissen from comment #8)
> Moreover, see the short discussion in Bug 566074, how old workbenches should
> be supported in automatic migrations?
I mean the discussion in Bug 565948
Comment 11 Christoph Laeubrich CLA 2021-07-13 10:11:06 EDT
(In reply to Rolf Theunissen from comment #8)
> Moreover, see the short discussion in Bug 566074, how old workbenches should
> be supported in automatic migrations?

That's the real point (for me) if I understand right this only happens when upgrading from a Workspace for an eclipse version around 4(!) years old, I won't expect any project to support that and simply say, there is either a way to reset the workspace xmi or one should simply create a new workspace when upgrading from such an old eclipse to the most recent one...

No idea if it would help to first upgrade the workspace with a 2018, 2019, 2020 ... eclipse probably work...
Comment 12 Andrey Loskutov CLA 2021-07-13 10:30:50 EDT
(In reply to Christoph Laeubrich from comment #11)
> (In reply to Rolf Theunissen from comment #8)
> > Moreover, see the short discussion in Bug 566074, how old workbenches should
> > be supported in automatic migrations?
> 
> That's the real point (for me) if I understand right this only happens when
> upgrading from a Workspace for an eclipse version around 4(!) years old

What is wrong with 4 old workspaces, why shouldn't they be supported? We have customers that expect the system is working for 10 years *at least*, so if they move to a "new" version, they will face exact this problem.

Even in our development lab, me and my colleague both have workspaces created *very* long time ago and we see this and the issue of bug 574805 now after moving to 4.21, despite the fact that we've used this workspaces with 4.9, 4.12, 4.15 versions. There is a reason workspaces are called so, they are *work* spaces. Why should we throw them away with every release, if we have lot of projects/settings etc unique for the concrete purpose of that workspace?

> I won't expect any project to support that 

How about programs that are supposed to read documents created a decade ago? Wouldn't you expect Word 2020 to still *read* the document created with Word 6? Why not? Why should Eclipse be different with workspace data? It should at least be able to properly read the old one, there is of course no need to write a compatible version back to the disk.

Even if we can't read our data back, we should not generate "broken" workspaces and simply ignore everything from the "unsupported" model version. Right now we don't care and produce some crazy models that are half way working. 

> and simply say, there is either a
> way to reset the workspace xmi or one should simply create a new workspace
> when upgrading from such an old eclipse to the most recent one...

I'm pretty sure the if we ask people around (those who aren't on CC for this bug), if they know what to do if views miss "close" buttons, they would have no idea - and if you proposes them to reset workspace xmi, they will still have no clue what do you want from them.

Please try to understand: not everyone who uses Eclipse is a Platform UI developer, it could be not even a developer at all, a test engineer etc.

> No idea if it would help to first upgrade the workspace with a 2018, 2019,
> 2020 ... eclipse probably work...

No, it will not, we see it in our "old" workspaces that were upgraded to 4.9, 4.12, 4.15.
Comment 13 Christoph Laeubrich CLA 2021-07-13 11:06:14 EDT
There is nothing wrong with old workspaces, and I don't suggested to delete them with every release. But if I need to reset my workspace/view after 4 years of usage that's much less effort than expecting (volunteer) developers to test with every combination of past eclipse version and workspace created in the past 4 years... The clear difference to Word 6 is that Microsoft gets lots of money supporting this use case.

I would expect from anyone coding with eclipse ide that he is capable of following a simple instruction like: 

1) navigate to <workspacefolder>/.metadata/whatever
2) delete (or rename) the file named workspace.xmi (or whatever file)

So if you are facing the issue, it would be good to simply check if such a thing would help, you should even be able to add the missing flags in the xmi to check if this is an alternative for a search+replace alternative solution ...
Comment 14 Andrey Loskutov CLA 2021-07-13 11:27:56 EDT
(In reply to Christoph Laeubrich from comment #13) 
> I would expect from anyone coding with eclipse ide that he is capable of
> following a simple instruction like: 
> 
> 1) navigate to <workspacefolder>/.metadata/whatever
> 2) delete (or rename) the file named workspace.xmi (or whatever file)

In case you knows whom to ask right question it could be a solution, but if you have simply no clue why out of the blue the close buttons are missing in the workspace, you will never came to the steps above. We would just have one angry user more.

Let's assume that this is a real problem for end users. Can we propose a solution that would NOT require any user interaction? Should we simply check workspace version and delete the xmi file on startup if the workspace is too old? Anything more elegant?
Comment 15 Christoph Laeubrich CLA 2021-07-13 11:56:54 EDT
(In reply to Andrey Loskutov from comment #14)
> Let's assume that this is a real problem for end users. Can we propose a
> solution that would NOT require any user interaction? Should we simply check
> workspace version and delete the xmi file on startup if the workspace is too
> old? Anything more elegant?

There is already a check for a new version/converting an old workspace, but I don't know if there are any "upgrad-hooks" that could be useful, still the first thing to check would be to see if deleting the xmi helps.

Alternatively an Upgrade-Check might add the missing items to the xmi directly but I'm not an E4Model expert to know how/if this is possible at all.
Comment 16 Andrey Loskutov CLA 2021-07-14 00:24:23 EDT
*** Bug 574836 has been marked as a duplicate of this bug. ***
Comment 17 Rolf Theunissen CLA 2021-07-14 02:36:23 EDT
(In reply to Andrey Loskutov from comment #14)
> (In reply to Christoph Laeubrich from comment #13) 
> > I would expect from anyone coding with eclipse ide that he is capable of
> > following a simple instruction like: 
> > 
> > 1) navigate to <workspacefolder>/.metadata/whatever
> > 2) delete (or rename) the file named workspace.xmi (or whatever file)
> 
> In case you knows whom to ask right question it could be a solution, but if
> you have simply no clue why out of the blue the close buttons are missing in
> the workspace, you will never came to the steps above. We would just have
> one angry user more.
> 
> Let's assume that this is a real problem for end users. Can we propose a
> solution that would NOT require any user interaction? Should we simply check
> workspace version and delete the xmi file on startup if the workspace is too
> old? Anything more elegant?

The instructions could be easier:
1) start once with -clearPersistedState command line argument

Given that this option exists, it must be possible to trigger this action for some old workspace too. Having no user interaction or just an 'ok' would be the best solution indeed.

Another option with less impact for the user, but more work for the developers, is to use model migrators. This are model processors that are used to cleanup or update the model to the new state. These migrators have been used before, and even some have been removed already. In those cases, if you don't make a jump from 4.4 to 4.17 directly it will work.

This is why I ask what is the (communicated) policy w.r.t. old workspace to the end-user, which scenarios are supposed to be supported? Given the limited resources, also here we might not be able to deliver the best experience to the user. Should the user expect that it is possible to import any 2.x,3.x workspace in any Eclipse 4.x version? Or would the user be advice to start a new workspace and import the old projects into that new workspace? Do we expect that if you create a workspace in 3.0 and open it in every 3.x and 4.x version (3.1, 3.2... 3.7, 4.1 ... 4.20) that it functions without problem on 4.21?

In the scope of this bug, Bug 516403 (4.8) should have provided a model migrator that added the tags and attributes to existing models (for Eclipse IDE only). But the leak of this migrator would only become apparent after Bug 433465 (4.17). So workspace models created in 4.7 or before will be broken in 4.17 and after. Adding a model migrator or (automatically) removing the workspace model now would only make old workspaces correct again in 4.21.
Comment 18 Rolf Theunissen CLA 2021-07-14 02:44:21 EDT
Unfortunately, we discovered this bug before Bug 567054. 

The I assumed it was caused by workspaces created in 3.x. It also lists some
workarounds:
1. start with "-clearPersistedState", this will remove the workspace model and reset all UI changes.
2a. set the closable attribute using the model-spy
2b. set the closable attribute in the workbench model by manually editing workspace.xmi (make sure u make backups)

Reset perspective might work as well.
Comment 19 Rolf Theunissen CLA 2021-07-14 02:44:54 EDT
*** Bug 567054 has been marked as a duplicate of this bug. ***
Comment 20 Rolf Theunissen CLA 2021-07-30 07:06:07 EDT
*** Bug 575128 has been marked as a duplicate of this bug. ***
Comment 21 Rolf Theunissen CLA 2021-07-31 14:50:16 EDT
(In reply to Rolf Theunissen from comment #7)
> (In reply to Andrey Loskutov from comment #4)
> > So in isClosable() 
> > 
> > https://git.eclipse.org/r/c/platform/eclipse.platform.ui/+/163756/4/bundles/
> > org.eclipse.e4.ui.workbench.renderers.swt/src/org/eclipse/e4/ui/workbench/
> > renderers/swt/StackRenderer.java
> > 
> > if we have *no* tags, can we assume the view is closeable?
> 
> No, we cannot

On second thought, indeed when the tags are not present then we can also assume that the closeable attribute was also not set. In general this will of course not be true, but in the IDE context I expect this to be true. Based on that assumption, we could create a model processor that corrects these missing tags and closable state.
Comment 22 Rolf Theunissen CLA 2021-08-02 03:05:31 EDT
(In reply to Rolf Theunissen from comment #18)
> Reset perspective might work as well.

For perspective that are not saved as snippets (the default ones), reset perspective works, as the whole perspective is re-created including placeholders.
For perspectives that are stored in snippets (that includes all custom saved ones) reset perspective doesn't work, as that takes a copy of that snippet, and the snippet contains the broken placeholders.
Comment 23 Deb Lewis CLA 2021-08-25 13:07:19 EDT
(In reply to Rolf Theunissen from comment #21)
> (In reply to Rolf Theunissen from comment #7)
> > (In reply to Andrey Loskutov from comment #4)
> > > So in isClosable() 
> > > 
> > > https://git.eclipse.org/r/c/platform/eclipse.platform.ui/+/163756/4/bundles/
> > > org.eclipse.e4.ui.workbench.renderers.swt/src/org/eclipse/e4/ui/workbench/
> > > renderers/swt/StackRenderer.java
> > > 
> > > if we have *no* tags, can we assume the view is closeable?
> > 
> > No, we cannot
> 
> On second thought, indeed when the tags are not present then we can also
> assume that the closeable attribute was also not set. In general this will
> of course not be true, but in the IDE context I expect this to be true.
> Based on that assumption, we could create a model processor that corrects
> these missing tags and closable state.

Some user feedback after running into this problem last month upgrading multiple workspaces to 4.20 release (reported in Bug 574836). Rolf's comments here and in Bug 567054 about adding closeable attribute on xsi:type="advanced:Placeholder" elements in workbench.xmi provided successful upgrade technique.

I had 17 workspaces that had all been used with eclipse 4.11 release from 2019; every one of them has perspective customizations, so brute force -clearPersistedState reset or having to rebuild every workspace would have been required significant amount of time to reconstruct.  Manual editing technique was tedious but with some regex hacking fairly predictable - hours rather than days to reconstruct.

Two workspaces known to have been created under 4.11 did not need patching - all placeholder elements had closeable attribute and child tag elements.  All other workspaces created under older versions and upgraded/used with 4.11 generally did not have closable/tags on "advanced:Placeholder" elements - regex search/replace to add closable everywhere (but not messing with tags) produced a functioning 4.20 workspace; have not so far seem any unexpected problems with the result.  Likely a model upgrade should have been performed by 4.11 but wasn't.

Christoph isn't wrong to ask how far back old workspace versions need to be supported e.g. I wouldn't really expect an old 3.x workspace to necessarily be usable if opened now by a current 4.x eclipse.  But my expectation as a long-time eclipse user is that versions from within the last few years should be upgradable.  Possibly a support/compatibility policy needs to be stated so users know what to expect.