Comment 1 Daniel Rolka 2013-12-17 08:53:58 EST

Gerrit review links: https://git.eclipse.org/r/#/c/19900/ and https://git.eclipse.org/r/#/c/19903/

Daniel

Comment 2 Paul Webster 2013-12-18 14:50:52 EST

I've released the CSSConstant support as http://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/?id=1e51dbbe2f05f1283ad283126979a7a0ae1b7667

For https://git.eclipse.org/r/#/c/19903/ when I copied the change into my e4_classic_winxp.css the text went grey when the workbench window de-activated, but did not turn back once the window became active.  I had extra to make that happen (see below).  Is that how it's supposed to be?  Is the CTabItem:selected rule really needed?

.MPartStack.active {
  swt-selected-tabs-background: #E5EDFC #99BAF3 100%;
  color: '#org-eclipse-ui-workbench-ACTIVE_TAB_TEXT_COLOR';
}

.MPartStack.active > CTabItem:selected {
	color: '#org-eclipse-ui-workbench-ACTIVE_TAB_TEXT_COLOR';
}


MPartStack.active.noFocus {	
	swt-selected-tabs-background: '#org-eclipse-ui-workbench-ACTIVE_NOFOCUS_TAB_BG_START' '#org-eclipse-ui-workbench-ACTIVE_NOFOCUS_TAB_BG_END' 100%;
	color: '#org-eclipse-ui-workbench-ACTIVE_NOFOCUS_TAB_TEXT_COLOR';
}

.MPartStack.active.noFocus > CTabItem:selected {
	color: '#org-eclipse-ui-workbench-ACTIVE_NOFOCUS_TAB_TEXT_COLOR';
}

Comment 3 Daniel Rolka 2013-12-19 06:12:23 EST

(In reply to Paul Webster from comment #2)
> I've released the CSSConstant support as
> http://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/
> ?id=1e51dbbe2f05f1283ad283126979a7a0ae1b7667
> 
> For https://git.eclipse.org/r/#/c/19903/ when I copied the change into my
> e4_classic_winxp.css the text went grey when the workbench window
> de-activated, but did not turn back once the window became active.  I had
> extra to make that happen (see below).  Is that how it's supposed to be?  Is
> the CTabItem:selected rule really needed?
> 
> .MPartStack.active {
>   swt-selected-tabs-background: #E5EDFC #99BAF3 100%;
>   color: '#org-eclipse-ui-workbench-ACTIVE_TAB_TEXT_COLOR';
> }
> 
> .MPartStack.active > CTabItem:selected {
> 	color: '#org-eclipse-ui-workbench-ACTIVE_TAB_TEXT_COLOR';
> }
> 
> 
> MPartStack.active.noFocus {	
> 	swt-selected-tabs-background:
> '#org-eclipse-ui-workbench-ACTIVE_NOFOCUS_TAB_BG_START'
> '#org-eclipse-ui-workbench-ACTIVE_NOFOCUS_TAB_BG_END' 100%;
> 	color: '#org-eclipse-ui-workbench-ACTIVE_NOFOCUS_TAB_TEXT_COLOR';
> }
> 
> .MPartStack.active.noFocus > CTabItem:selected {
> 	color: '#org-eclipse-ui-workbench-ACTIVE_NOFOCUS_TAB_TEXT_COLOR';
> }

Yes, we need the 'CTabItem:selected' selector to properly style the selected MPart in all MPartStacks.

The patch depends on the https://git.eclipse.org/r/#/c/18383/9 one that is also under review. Have you applied the patch https://git.eclipse.org/r/#/c/18383/9 first and next the https://git.eclipse.org/r/#/c/19903/ one in your environment? 
It works fine after applying both patches in the Windows 7 without any additional change in the CSS stylesheet

Daniel

Comment 4 Paul Webster 2013-12-24 10:51:36 EST

(In reply to Daniel Rolka from comment #3)
> The patch depends on the https://git.eclipse.org/r/#/c/18383/9 one that is
> also under review. Have you applied the patch
> https://git.eclipse.org/r/#/c/18383/9 first and next the
> https://git.eclipse.org/r/#/c/19903/ one in your environment? 
> It works fine after applying both patches in the Windows 7 without any
> additional change in the CSS stylesheet

I extracted just 3 CSS rules, so maybe I was missing others defined in the rest of your patch.

PW

Comment 5 Eric Moffatt 2014-02-28 10:15:22 EST

Committed:

http://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/?id=e6b0d4c880eacc1607e7c31697ed642c63b9947e

This safes up the setCSSInfo class by checking for null widgets. While I was testing the fix for Bug 394394 I ended up in a state where I was getting NPE's from the new 'updateNonFocusState' code which was passing 'stack.getWidget()'. Sorry, I don't have solid repro steps but I was playing around with closing the last view in stacks with various states (mostly maximized). 

Rather than fix the code in WBWRenderer I just safe'd up the code in setCSSInfo to do the check. This is a good idea in any case IMO...

Comment 6 Daniel Rolka 2014-03-24 07:38:46 EDT

The proper patch has been already committed to master

Daniel