Bugzilla – Full Text Bug Listing
| Summary: | [Intro] Welcome screen gets pushed to the right side unless Problems view is on top | ||
|---|---|---|---|
| Product: | [Eclipse Project] Platform | Reporter: | Julia Perdigueiro <julia.perdigueiro> |
| Component: | UI | Assignee: | Dani Megert <daniel_megert> |
| Status: | RESOLVED FIXED | QA Contact: | Eric Moffatt <emoffatt> |
| Severity: | normal | ||
| Priority: | P3 | CC: | daniel_megert, ericc, jugujoo, markusb, pwebster, remy.suen |
| Version: | 3.7 | ||
| Target Milestone: | 3.8 M4 | ||
| Hardware: | PC | ||
| OS: | Windows 7 | ||
| Whiteboard: | |||
|
Description
Julia Perdigueiro
Reproduced on my Windows XP workstation. I'm suspecting based on the problem being specifically regarding the Problems view (a marker view) and only occurring when it's *not* on top that this may have been introduced by the fix for bug 294959. I'll comment on that defect to see what we can come up with... Adding Dani for his take... Here's the trace from when it happens... Thread [main] (Suspended (breakpoint at line 185 in WorkbenchIntroManager)) WorkbenchIntroManager.setIntroStandby(IIntroPart, boolean) line: 185 WorkbenchPage.checkIntro() line: 1198 WorkbenchPage.busyShowView(IViewPart, int) line: 1168 WorkbenchPage.busyShowView(String, String, int) line: 1140 WorkbenchPage$20.run() line: 3921 BusyIndicator.showWhile(Display, Runnable) line: 70 WorkbenchPage.showView(String, String, int) line: 3918 IDEWorkbenchPlugin$2.run() line: 382 Display.runTimer(int) line: 4264 ... The timer goes off and opens the ProblemsView in 'CREATE' mode The 'busyShowView' calls 'checkIntro' which *always* turns it into standby mode if it's not already there. The likely fix is to change 'busyShowIntro' to check the creation mode and if it's 'create' to *not* call 'checkIntro' (since simple creation doesn't imply either activation or bring to top... (In reply to comment #2) > I'm suspecting based on the problem being specifically regarding the Problems > view (a marker view) and only occurring when it's *not* on top that this may > have been introduced by the fix for bug 294959. It's not caused by that bug but by the fix for bug 329002. What we do there is: activePage.showView(viewReference.getId(), viewReference.getSecondaryId(), IWorkbenchPage.VIEW_CREATE); It looks like for some reason the VIEW_CREATE has side effects on the Welcome page (which it shouldn't) - but that could also be caused by wrong code in the Welcome page: Only the Welcome page is affected, other views and editors that are maximized are not put aside on restart. On the other hand, the Welcome page seems to have code that puts other views aside: if one has the Welcome page in the window and some other maximized while restarting, the Welcome page shows itself, putting the maximized part aside. > The likely fix is to change 'busyShowIntro' to check the creation mode and if
> it's 'create' to *not* call 'checkIntro' (since simple creation doesn't imply
> either activation or bring to top...
Correct. I'm already testing that fix.
Eric, I'll fix it. Please add a test case to the test suite to prevent regressions. Fixed in R3_development: f78f98290ccff19750795bdd3b63b5c2bb22d26b It doesn't seen to be an issue in 4.x (and there is not Welcome yet). (In reply to comment #8) > Please add a test case to the test suite to prevent regressions. Remy, do we already have tests that test Welcome page/editor state after restart? If so, can you point me to them? If there isn't already a template I'm afraid I won't have time to invest into a complicated test for this one liner 3.x fix. > It doesn't seen to be an issue in 4.x (and there is not Welcome yet). Filed bug 363923 to track that. Dani, I'm doing the 4.x Intro work now. Let me know if the proposed fix works and I'll make the same change in the 4.x stream as part of it... (In reply to comment #12) > Dani, I'm doing the 4.x Intro work now. Let me know if the proposed fix works > and I'll make the same change in the 4.x stream as part of it... Good to hear. If you add something similar to org.eclipse.ui.intro.IIntroManager.setIntroStandby(IIntroPart, boolean).checkIntro() then just protect it the same way as we now do in 3.x. Also, only if you add a call to IIntroManager.setIntroStandby(anIntroPart, true) you have to worry about it. > Remy, do we already have tests that test Welcome page/editor state after
> restart? If so, can you point me to them? If there isn't already a template I'm
> afraid I won't have time to invest into a complicated test for this one liner
> 3.x fix.
Remy, any hint? If not, I'm going to mark this FIXED and open a new bug for the test, in case someone finds time for it at later point.
(In reply to comment #14) > Remy, any hint? If not, I'm going to mark this FIXED and open a new bug for the > test, in case someone finds time for it at later point. I saw an IntroTest yesterday which seems to check the standby state of the intro. I wasn't able to actually run the test though so I haven't verified whether this was true or not. If you want to mark this fixed then that's fine with me. > I saw an IntroTest yesterday which seems to check the standby state of the > intro. I wasn't able to actually run the test though so I haven't verified > whether this was true or not. Checking the state is not so the problem but rather setting up a state, restart and then check that state again. > If you want to mark this fixed then that's fine > with me. OK, thx. (In reply to comment #16) > > I saw an IntroTest yesterday which seems to check the standby state of the > > intro. I wasn't able to actually run the test though so I haven't verified > > whether this was true or not. > Checking the state is not so the problem but rather setting up a state, restart > and then check that state again. But couldn't you check that it is in fullscreen mode, show a view with IWorkbenchPage.VIEW_CREATE and then check that it's still in fullscreen mode? I don't think restarting is a necessity here. (In reply to comment #17) > (In reply to comment #16) > > > I saw an IntroTest yesterday which seems to check the standby state of the > > > intro. I wasn't able to actually run the test though so I haven't verified > > > whether this was true or not. > > Checking the state is not so the problem but rather setting up a state, restart > > and then check that state again. > > But couldn't you check that it is in fullscreen mode, show a view with > IWorkbenchPage.VIEW_CREATE and then check that it's still in fullscreen mode? I > don't think restarting is a necessity here. I was hoping for a test that verified comment 0. Adding a test as you mentioned is easy. Will do that for you :-). Added two tests: one that creates the Problems view while Welcome is in front and one that opens and activates the Problems view. Fixed in R3_development: 502b10a501707327c51842b9ad9d87ab7d2a7c15 Eric or Remy: ui.tests is not yet split i.e. nothing to do in 'master' and the changes will be copied over to 'master' like all the other changes in non-split bundles, right? (In reply to comment #19) > Eric or Remy: ui.tests is not yet split i.e. nothing to do in 'master' and the > changes will be copied over to 'master' like all the other changes in non-split > bundles, right? I don't believe we have this automated. (In reply to comment #19) > Added two tests: one that creates the Problems view while Welcome is in front > and one that opens and activates the Problems view. > > Fixed in R3_development: 502b10a501707327c51842b9ad9d87ab7d2a7c15 > > > Eric or Remy: ui.tests is not yet split i.e. nothing to do in 'master' and the > changes will be copied over to 'master' like all the other changes in non-split > bundles, right? Daniel, thanks for the quick fix. Now, is there a way to get a patch we could use for an existing Indigo version that would just fix this behavior? Or are there too many dependencies? Thanks Markus (In reply to comment #20) > (In reply to comment #19) > > Eric or Remy: ui.tests is not yet split i.e. nothing to do in 'master' and the > > changes will be copied over to 'master' like all the other changes in non-split > > bundles, right? > > I don't believe we have this automated. OK. So are you saying the non-split branches in master are basically dead wood? Because AFAIK developers did not have to cherry-pick fixes in non-shared bundles. (In reply to comment #21) > Daniel, thanks for the quick fix. Now, is there a way to get a patch we could > use for an existing Indigo version that would just fix this behavior? Or are > there too many dependencies? > > Thanks > Markus Is this really a frequent use case? I doubt it. Paul should decide whether he wants to backport this or not. (In reply to comment #23) > (In reply to comment #21) > > Daniel, thanks for the quick fix. Now, is there a way to get a patch we could > > use for an existing Indigo version that would just fix this behavior? Or are > > there too many dependencies? > > > > Thanks > > Markus > > Is this really a frequent use case? I doubt it. Paul should decide whether he > wants to backport this or not. No, that's fine. If it would have been an easy, quick, risk-free (I know, there's no such thing;-) exchange of a class we might have considered it. (In reply to comment #22) > (In reply to comment #20) > > (In reply to comment #19) > > > Eric or Remy: ui.tests is not yet split i.e. nothing to do in 'master' and the > > > changes will be copied over to 'master' like all the other changes in non-split > > > bundles, right? > > > > I don't believe we have this automated. > > OK. So are you saying the non-split branches in master are basically dead wood? > Because AFAIK developers did not have to cherry-pick fixes in non-shared > bundles. Ah, I remember now: the 'master' branch was auto-filled but fixes had to be manually cherry-picked. Did that now for the tests. master commit: 09220d483db2705d8462ed6bc09d5761589c12f6 *** Bug 353837 has been marked as a duplicate of this bug. *** |