Bug 574805 - Shared views have missing icons for workspace created with Eclipse 4.2
Summary: Shared views have missing icons for workspace created with Eclipse 4.2
Status: NEW
Alias: None
Product: Platform
Classification: Eclipse Project
Component: UI (show other bugs)
Version: 4.17   Edit
Hardware: PC All
: P3 normal (vote)
Target Milestone: ---   Edit
Assignee: Platform-UI-Inbox CLA
QA Contact:
URL:
Whiteboard:
Keywords: regression
Depends on:
Blocks:
 
Reported: 2021-07-12 11:21 EDT by Simeon Andreev CLA
Modified: 2021-07-13 04:31 EDT (History)
2 users (show)

See Also:


Attachments
Screenshot showing the problem. (47.09 KB, image/png)
2021-07-12 11:21 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-12 11:21:02 EDT
Created attachment 286771 [details]
Screenshot showing the problem.

To reproduce, create a new workspace with Eclipse 4.2, e.g. build: I20120608-1400

Then, start Eclipse 4.21 in that workspace (confirm that you will be using an older workspace). Observe that shared views (inactive view tabs) have missing icons, see attached screenshot "missing_icons_shared_views.png").

I cannot reproduce this with 4.15, will look for the first 4.x minor version that broke the icons.

Can be reproduced also by renaming icons from .png to .gif in the workbench.xmi file. Its enough to have e.g. the following in the workbench.xmi file:

    <sharedElements xsi:type="basic:Part" xmi:id="_5f0NsOMfEeuIUcwnzrzK2A" elementId="org.eclipse.ui.views.ProblemView" contributionURI="bundleclass://org.eclipse.ui.workbench/org.eclipse.ui.internal.e4.compatibility.CompatibilityView" label="Problems" iconURI="platform:/plugin/org.eclipse.ui.ide/icons/full/eview16/problems_view.gif" tooltip="" closeable="true">
      <persistedState key="originalCompatibilityViewClass" value="org.eclipse.ui.internal.views.markers.ProblemsView"/>
      <persistedState key="originalCompatibilityViewBundle" value="org.eclipse.ui.ide"/>
      <persistedState key="memento" value="&lt;?xml version=&quot;1.0&quot; encoding=&quot;UTF-8&quot;?>&#xA;&lt;view PRIMARY_SORT_FIELD=&quot;org.eclipse.ui.ide.severityAndDescriptionField&quot; categoryGroup=&quot;org.eclipse.ui.ide.severity&quot; markerContentGenerator=&quot;org.eclipse.ui.ide.problemsGenerator&quot; partName=&quot;Problems&quot;>&#xA;&lt;columnWidths org.eclipse.ui.ide.locationField=&quot;105&quot; org.eclipse.ui.ide.markerType=&quot;105&quot; org.eclipse.ui.ide.pathField=&quot;140&quot; org.eclipse.ui.ide.resourceField=&quot;105&quot; org.eclipse.ui.ide.severityAndDescriptionField=&quot;350&quot;/>&#xA;&lt;visible IMemento.internal.id=&quot;org.eclipse.ui.ide.severityAndDescriptionField&quot;/>&#xA;&lt;visible IMemento.internal.id=&quot;org.eclipse.ui.ide.resourceField&quot;/>&#xA;&lt;visible IMemento.internal.id=&quot;org.eclipse.ui.ide.pathField&quot;/>&#xA;&lt;visible IMemento.internal.id=&quot;org.eclipse.ui.ide.locationField&quot;/>&#xA;&lt;visible IMemento.internal.id=&quot;org.eclipse.ui.ide.markerType&quot;/>&#xA;&lt;/view>"/>
      <tags>View</tags>
      <tags>categoryTag:General</tags>
      <menus xmi:id="_6kAOkOMfEeuIUcwnzrzK2A" elementId="org.eclipse.ui.views.ProblemView">
        <tags>ViewMenu</tags>
        <tags>menuContribution:menu</tags>
      </menus>
      <toolbar xmi:id="_6kAOkeMfEeuIUcwnzrzK2A" elementId="org.eclipse.ui.views.ProblemView" visible="false"/>
    </sharedElements>
Comment 1 Andrey Loskutov CLA 2021-07-12 11:48:19 EDT
Rolf, could it be somehow related to bug 572598 (that is one where Shared parts are touched), or to bug 566074?
Comment 2 Simeon Andreev CLA 2021-07-12 11:56:25 EDT
I cannot reproduce with I20200609-1800, but can reproduce with I20200615-1800 (both are Eclipse 4.17).
Comment 4 Andrey Loskutov CLA 2021-07-12 12:16:33 EDT
(In reply to Andrey Loskutov from comment #3)
> Affected icons were removed in 4.17 via bug 550832 comment 13, commit
> http://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/
> ?id=30a3908271366b21ec15e2f5de4b77a91e33089d

Please note: JUnit view is also affected, among probably many other views. That was most likely done by later commits linked to same bug.

What really annoys me is that if the view with broken icon is activated, it immediately changes the icon to the right one, but this change is *not persisted back* (or not properly), so on next restart same issue re-appears.

Multiple ways to fix that:

1) We could check for "png" variant if we don't find "gif" (in SWTPartRenderer.getImageFromURI(String)?),
2) or properly save the model after we've opened the view (which seem not the case now, but I have no idea about this code), 
3) or simply add removed *view* icons back (not all gifs were from views).

Unless anyone has a better approach, I would just do 3). There were no good reason to delete old images anyway, concerns were raised, but the proposed workaround in bug 550832 comment 6 is not working in reality.
Comment 5 Simeon Andreev CLA 2021-07-13 02:14:45 EDT
> 1) We could check for "png" variant if we don't find "gif" (in 
> SWTPartRenderer.getImageFromURI(String)?),

IMO better to restore the icons. Especially since someone could have also renamed icons, in which case looking for a different file type won't help. I don't think this is the way to go for this ticket.

> 2) or properly save the model after we've opened the view (which seem not the 
> case now, but I have no idea about this code),

Do we want a separate bug to track this problem? To me seeing the red icons at any point is not acceptable behaviour, even if they would get fixed upon activating the view. I don't think this bug should track the defect in the model persistence.

Considering that paths to icons can change at any point, does it even make sense to persist the icon paths at all? Is there benefit to this, or is the model defined so that an icon path must be provided (and so it will be a wrong icon path, sooner or later)? IMO best to discuss this in another ticket.
Comment 6 Andrey Loskutov CLA 2021-07-13 02:25:48 EDT
(In reply to Simeon Andreev from comment #5)
> > 2) or properly save the model after we've opened the view (which seem not the 
> > case now, but I have no idea about this code),
> 
> Do we want a separate bug to track this problem? To me seeing the red icons
> at any point is not acceptable behaviour, even if they would get fixed upon
> activating the view. I don't think this bug should track the defect in the
> model persistence.

If someone who knows the code would fix this model loading/persistence, why not on this bug? It would be "fixed" by next startup then. 

> Considering that paths to icons can change at any point, does it even make
> sense to persist the icon paths at all? Is there benefit to this, or is the
> model defined so that an icon path must be provided (and so it will be a
> wrong icon path, sooner or later)? IMO best to discuss this in another
> ticket.

The benefit is lazy loading of the workbench without initialization or re-parsing contributions of *all* bundles contributed all the views in the current perspective (also inactive, minimized). 

As said, I would simply restore view icons. This is the simplest, less invasive fix that would also fix all other issues with code that assumed icon paths to exist and fails now. E.g. plugins can refer to icons from other plugins in declarative way in plugin.xml, but also code could do same - I've seen few plugins failing to load something because of icon removal.
Comment 7 Rolf Theunissen CLA 2021-07-13 04:31:20 EDT
That the icon is corrected when a part gets activated is probably due to the CompatiblityPart code. In its create() method it sets an override icon in the transient data of a part, i.e. it sets IPresentationEngine.OVERRIDE_ICON_IMAGE_KEY. This transient data is not stored, so that explains why it doesn't persist.

SWTPartRenderer#getImage determines the image that is used for a tab, it uses the 
IPresentationEngine.OVERRIDE_ICON_IMAGE_KEY when provided. Otherwise it falls back to the provided URI. Only if the iconURI is not set, it is falls back to the URI from the part descriptor. This behavior is changed in Bug 475357, before that the URI of the part descriptor was always used. This probably adds to this regression. Although I did not validate what icon URIs are stored in the PartDescriptors

A model migrator could be used to remove/correct the references to the broken icons in Parts and/or PartDescriptors. Though it is hard to quickly investigate how much effort that is.