Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.
Bug 334362 - Properties dialog: properties for closed project are sometimes wrong
Summary: Properties dialog: properties for closed project are sometimes wrong
Status: RESOLVED FIXED
Alias: None
Product: Platform
Classification: Eclipse Project
Component: IDE (show other bugs)
Version: 3.6   Edit
Hardware: All All
: P3 normal (vote)
Target Milestone: 3.7 RC1   Edit
Assignee: Szymon Ptaszkiewicz CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-01-14 05:13 EST by Dani Megert CLA
Modified: 2011-05-09 05:36 EDT (History)
3 users (show)

See Also:
daniel_megert: review+
serge: review+


Attachments
Patch v.0.1 (3.92 KB, patch)
2011-01-17 08:04 EST, Szymon Ptaszkiewicz CLA
no flags Details | Diff
.project for closed project (2.46 KB, image/jpeg)
2011-01-18 08:12 EST, Szymon Ptaszkiewicz CLA
no flags Details
Patch v.0.2 (6.54 KB, patch)
2011-01-24 05:12 EST, Szymon Ptaszkiewicz CLA
daniel_megert: iplog+
daniel_megert: review+
Details | Diff
Fix for regression introduced in patch v.0.2 (2.98 KB, patch)
2011-05-06 10:42 EDT, Szymon Ptaszkiewicz CLA
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Dani Megert CLA 2011-01-14 05:13:13 EST
N20110113-2000 but also in previous releases.

Some of the properties are wrong for the closed project after restart because the preference node is not available. Instead of showing wrong values, we should simply not show those sections (encoding, line delimiter) if the preference node is not available.
Comment 1 Szymon Ptaszkiewicz CLA 2011-01-14 06:02:03 EST
I agree. What about other pages (Project References, Run/Debug Settings)? If we cannot change anything there because the project is closed, maybe we should hide them as well?
Comment 2 Dani Megert CLA 2011-01-14 06:11:25 EST
(In reply to comment #1)
> I agree. What about other pages (Project References, Run/Debug Settings)? If we
> cannot change anything there because the project is closed, maybe we should
> hide them as well?
Yes, but only if they don't show the right information. The project refs for example seem to be correct.
Comment 3 Dani Megert CLA 2011-01-14 06:12:00 EST
(In reply to comment #2)
> (In reply to comment #1)
> > I agree. What about other pages (Project References, Run/Debug Settings)? If we
> > cannot change anything there because the project is closed, maybe we should
> > hide them as well?
> Yes, but only if they don't show the right information. The project refs for
> example seem to be correct.

And: we already do this for other pages i.e. we don't even show the page name on the left (test with Java project).
Comment 4 Szymon Ptaszkiewicz CLA 2011-01-17 08:04:36 EST
Created attachment 186902 [details]
Patch v.0.1
Comment 5 Szymon Ptaszkiewicz CLA 2011-01-17 08:30:17 EST
(In reply to comment #3)
> (In reply to comment #2)
> > (In reply to comment #1)
> > > I agree. What about other pages (Project References, Run/Debug Settings)? If we
> > > cannot change anything there because the project is closed, maybe we should
> > > hide them as well?
> > Yes, but only if they don't show the right information. The project refs for
> > example seem to be correct.
> 
> And: we already do this for other pages i.e. we don't even show the page name
> on the left (test with Java project).

That's true, but what I meant is that Project References shows information that is stored in .project. If a project is closed, then Project References shows just a list of all projects in workspace (except for the current one). The list does not reflect current references stored in .project. Run/Debug Settings is similar - if the project is closed, you will not see launch configurations for the project.

I think that not showing correct information (project references, launch configurations) could be considered as showing wrong information as well. Dani, what do you think?
Comment 6 Dani Megert CLA 2011-01-18 02:31:17 EST
> That's true, but what I meant is that Project References shows information that
> is stored in .project. If a project is closed, then Project References shows
> just a list of all projects in workspace (except for the current one). The list
> does not reflect current references stored in .project. Run/Debug Settings is
> similar - if the project is closed, you will not see launch configurations for
> the project.
Correct. I didn't look close enough.
> 
> I think that not showing correct information (project references, launch
> configurations) could be considered as showing wrong information as well. Dani,
> what do you think?
I agreed. I think we should also hide those pages like we do it for most others.
Comment 7 Szymon Ptaszkiewicz CLA 2011-01-18 08:12:05 EST
Created attachment 186994 [details]
.project for closed project

I found one more issue: there are two child pages of Resources page and it is possible to edit properties on those pages for closed project. Those pages should also be hidden, but it is possible that there is a bug also in Platform/Resources as after closing the project, you can still see properties from .project and that should not be possible (see Resource Filters). It looks that those values are cached somewhere and not taken directly from .project. Moreover, if you change some values for closed project on those pages, you can receive .project for closed project (see attached image). I think we should first disable all widgets on those two pages and then hide those pages so that we have some consistency with previous fixes (see bug 157793).
Comment 8 Dani Megert CLA 2011-01-19 11:26:50 EST
(In reply to comment #7)
> I think we should
> first disable all widgets on those two pages and then hide those pages so that
> we have some consistency with previous fixes (see bug 157793).
Why would you care to disable widgets for a hidden page?

I'm fine leaving the pages but disable the controls.
Comment 9 Szymon Ptaszkiewicz CLA 2011-01-21 12:07:46 EST
(In reply to comment #8)
> I'm fine leaving the pages but disable the controls.

I'm confused. So you would like to hide all pages but for child pages of Resource you would prefer to disable controls. Isn't that conflicting with comment 6? I'd rather disable all pages except for Resource.
Comment 10 Dani Megert CLA 2011-01-21 12:32:48 EST
(In reply to comment #9)
> (In reply to comment #8)
> > I'm fine leaving the pages but disable the controls.
> 
> I'm confused. So you would like to hide all pages but for child pages of
> Resource you would prefer to disable controls. Isn't that conflicting with
> comment 6? I'd rather disable all pages except for Resource.
I would only show the (child) pages if the disabled data is still valid.
Comment 11 Szymon Ptaszkiewicz CLA 2011-01-24 05:12:03 EST
Created attachment 187405 [details]
Patch v.0.2
Comment 12 Szymon Ptaszkiewicz CLA 2011-01-24 05:18:03 EST
Dani, please review.
Comment 13 Dani Megert CLA 2011-01-25 11:28:36 EST
(In reply to comment #12)
> Dani, please review.

Looks good to me. I'll commit the patch once M5 has shipped.
Comment 14 Dani Megert CLA 2011-01-31 04:04:28 EST
Committed patch to HEAD.
Comment 15 Dani Megert CLA 2011-03-15 09:20:57 EDT
Verified in I20110310-1119.
Comment 16 Serge Beauchamp CLA 2011-05-06 05:05:07 EDT
This fix introduces a new bug:  The property pages do not show anymore for selection that are not IProjects.

Use case:

1) Right-click on a regular folder, and select 'properties'.
2) Verify that the property page 'Resource Filters' isn't visible.
Comment 17 Dani Megert CLA 2011-05-06 05:50:59 EDT
Szymon, this needs to be addressed for RC1.
Comment 18 Szymon Ptaszkiewicz CLA 2011-05-06 10:42:50 EDT
Created attachment 194937 [details]
Fix for regression introduced in patch v.0.2
Comment 19 Szymon Ptaszkiewicz CLA 2011-05-06 10:45:45 EDT
Serge, thanks for letting us know about this problem.
Dani, Serge, could you review the new patch?
Comment 20 Serge Beauchamp CLA 2011-05-06 10:56:59 EDT
I reviewed the changes and tested the use case, and it works well.

Thanks for the quick turn around.
Comment 21 Dani Megert CLA 2011-05-09 05:36:30 EDT
Thanks Szymon - the fix looks good.
Committed the patch to HEAD.