Bugzilla – Full Text Bug Listing
| Summary: | Properties dialog: properties for closed project are sometimes wrong | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | [Eclipse Project] Platform | Reporter: | Dani Megert <daniel_megert> | ||||||||||
| Component: | IDE | Assignee: | Szymon Ptaszkiewicz <sptaszkiewicz> | ||||||||||
| Status: | RESOLVED FIXED | QA Contact: | |||||||||||
| Severity: | normal | ||||||||||||
| Priority: | P3 | CC: | ob1.eclipse, serge, Szymon.Brandys | ||||||||||
| Version: | 3.6 | Flags: | daniel_megert:
review+
serge: review+ |
||||||||||
| Target Milestone: | 3.7 RC1 | ||||||||||||
| Hardware: | All | ||||||||||||
| OS: | All | ||||||||||||
| Whiteboard: | |||||||||||||
| Attachments: |
|
||||||||||||
|
Description
Dani Megert
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? (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. (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). Created attachment 186902 [details]
Patch v.0.1
(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? > 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. 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). (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. (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. (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. Created attachment 187405 [details]
Patch v.0.2
Dani, please review. (In reply to comment #12) > Dani, please review. Looks good to me. I'll commit the patch once M5 has shipped. Committed patch to HEAD. Verified in I20110310-1119. 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. Szymon, this needs to be addressed for RC1. Created attachment 194937 [details]
Fix for regression introduced in patch v.0.2
Serge, thanks for letting us know about this problem. Dani, Serge, could you review the new patch? I reviewed the changes and tested the use case, and it works well. Thanks for the quick turn around. Thanks Szymon - the fix looks good. Committed the patch to HEAD. |