Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.

Bug 354739

Summary:

[Compatibility] Perspectives with standalone views lay out incorrectly

Product:

[Eclipse Project] Platform

Reporter:

Ben Roling <rollsisu>

Component:

Assignee:

Remy Suen <remy.suen>

Status:

RESOLVED FIXED

QA Contact:

Remy Suen <remy.suen>

Severity:

normal

Priority:

CC:

emoffatt, ob1.eclipse, pwebster, remy.suen

Version:

4.1

Flags:

emoffatt: review+
ob1.eclipse: review+

Target Milestone:

4.1.1

Hardware:

All

OS:

All

Whiteboard:

Attachments:

Description	Flags
screenshot when views are "normal" (non-standalone) views	none
screenshot of standalone views displaying in wrong positions	none
patch against RCP mail demo app used to demonstrate the issue	none
ModeledPageLayout patch v1	none

Description Ben Roling

2011-08-15 11:22:00 EDT

Build Identifier: I20110620-1631

In the process of testing conversion of an Eclipse 3.6 based RCP app to E4, I came across an issue where a perspective made of of standalone views does not lay out correctly.  The views show up in the wrong positions.  If the views are switched to "normal" (non-standalone) views, then they show up in the correct positions.

I am attaching screenshots and a patch against the RCP mail demo application that demonstrate the issue.

Reproducible: Always

Steps to Reproduce:
1. Create a perspective with a layout of 4 standalone views as demonstrated in the attached patch
2. Run the application and open the perspective from step 1
3. Observe the views being placed into the incorrect positions

Comment 1 Ben Roling

2011-08-15 11:22:51 EDT

Created attachment 201501 [details]
screenshot when views are "normal" (non-standalone) views

Comment 2 Ben Roling

2011-08-15 11:23:28 EDT

Created attachment 201502 [details]
screenshot of standalone views displaying in wrong positions

Comment 3 Ben Roling

2011-08-15 11:25:36 EDT

Created attachment 201504 [details]
patch against RCP mail demo app used to demonstrate the issue

In the attached patch, Perspective2 contains the layout demonstrating the issue.  There is a block that can be uncommented to switch between standalone and "normal" views.

Comment 4 Remy Suen

2011-08-15 11:47:57 EDT

Confirmed to be a problem.

Thank you for testing Eclipse 4, Ben. In the future, you can just open bugs encountered with the compatibility layer in Platform UI with a 4.x version.

Comment 5 Ben Roling

2011-08-15 12:10:24 EDT

Sure - thanks for the quick confirmation Remy.

Comment 6 Remy Suen

2011-08-15 14:12:26 EDT

Created attachment 201510 [details]
ModeledPageLayout patch v1

Change the stacking code to consider the part itself if the part's parent is not a part stack.

Comment 7 Remy Suen

2011-08-15 14:12:58 EDT

(In reply to comment #6)
> Created attachment 201510 [details]
> ModeledPageLayout patch v1

Eric, could you review this patch?

Comment 8 Remy Suen

2011-08-16 11:22:44 EDT

(In reply to comment #6)
> Created attachment 201510 [details]
> ModeledPageLayout patch v1

The gist of the problem is that we were _always_ inserting elements relative to a part's parent. This meant that parent part sash containers were being considered if the part was not in a part stack (for standalone views). This is not correct as the item should just be inserted relative to the part instead if it is not in a part stack. We do want it to be inserted relative to a part stack if the part is in a part stack as we would otherwise be introducing shapes inside a part stack (when it only supports parts).

Think that explanation might've been a little confusing...

Comment 9 Ben Roling

2011-08-17 12:53:14 EDT

FYI - I tested out the patch and it looks to me like it resolves the issue.  Thanks for the very quick turnaround.  Hopefully Eric or someone else can get it reviewed and you can incorporate it into the code base.

Comment 10 Eric Moffatt

2011-08-22 14:04:31 EDT

Adding Oleg for addition review...

Comment 11 Eric Moffatt

2011-08-22 14:24:23 EDT

This looks good. Only comment is that we likely want to make the existing code for creating a view *in a stack* positioned relative to a standalone view it should use the new 'findRefModel' code...

I tested this using a non-RCP project to define the new Perspective...it was easier than doing the extra work to make the RCP Mail app run...;-).

Comment 12 Oleg Besedin

2011-08-22 14:26:37 EDT

+1, looks good to me.

Comment 13 Remy Suen

2011-08-22 15:05:23 EDT

(In reply to comment #11)
> Only comment is that we likely want to make the existing code
> for creating a view *in a stack* positioned relative to a standalone view it
> should use the new 'findRefModel' code...

Makes sense to me, opened bug 355426 for that.

Pushed the fix to R4_development.
http://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/?h=R4_development&id=57f210854459fb8f40a07f1f2f76fa9cca44ed74

Pushed too much stuff so I reverted parts of it.
http://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/?h=R4_development&id=bdda1c43198d357961f1979c6cc398c9ae49fb29

Thanks Eric and Oleg for the review! And thank you Ben for reporting the bug and checking the patch!

Comment 14 Ben Roling

2011-09-21 12:12:03 EDT

Will 4.1.1 be released at the end of this month as suggested by the Indigo SR1 Endgame Plan [1]?  Sorry if that's a dumb question but I'm not exactly sure on how to tell whether or not its still on schedule.  This particular bug fix is important to me and I would be happy if it was going to be in the service release and the service release was going to be soon.

Thanks!

[1] http://www.eclipse.org/eclipse/development/plans/freeze_plan_4_1_1.php

Comment 15 Remy Suen

2011-09-21 12:17:03 EDT

(In reply to comment #14)
> Will 4.1.1 be released at the end of this month as suggested by the Indigo SR1
> Endgame Plan [1]?

Yes, we plan to have a 4.1.1 at the same time as 3.7.1. You can download one of the interim maintenance builds for now if you wish.

Comment 16 Ben Roling

2011-09-21 12:42:08 EDT

Great, thanks again!