Community
Participate
Working Groups
We need a Shell:active pseudo class so that we can properly describe the CTabFolder styling behaviour you see in Eclipse 3.x, ie: Shell:active CTabFolder:focus CTabItem:selected { ... }
Created attachment 126305 [details] shell: active now works styles such as the following won't work: Shell:active CTabFolder{ (whatever styling) }
(In reply to comment #1) > Created an attachment (id=126305) [details] > shell: active now works Great! > styles such as the following won't work: > > Shell:active CTabFolder{ > (whatever styling) > } Do we understand why not? ie. is this just an incorrect expectation of what we can do in CSS, or something missing or bug in our support?
(In reply to comment #2) > Do we understand why not? ie. is this just an incorrect expectation of what we > can do in CSS, or something missing or bug in our support? Like you said, I think it may be an incorrect expectation of our CSS Engine.
(In reply to comment #3) > (In reply to comment #2) > > > Do we understand why not? ie. is this just an incorrect expectation of what we > > can do in CSS, or something missing or bug in our support? > > Like you said, I think it may be an incorrect expectation of our CSS Engine. Sorry, I'm asking if: A) You could never write something like this in CSS for a web browser, it's the wrong format in CSS. B) Our engine isn't handling it correctly.
Please add junit test to patch (or separate patch). Note that you will likely need to open() the shell for this test to work. Therefore, it should be a new test. Try using forceActive() to get it in the active state. To test the non-active state will require more cleverness. Perhaps make/open a second shell and make *it* active, thus ensuring the test shell isn't.
Created attachment 126735 [details] junit for shell:active
Test doesn't seem to work. Note that the patch didn't included adding the test to the test suite so maybe you thought it ran successfully when in fact it was never run? I'm on Vista so maybe that also makes a difference? I tried adding shell.forceActive() but that didn't help. It may not be possible to automate the testing for all platforms. Can you confirm that by adding this test to the suite that it in fact passes?
(In reply to comment #) > Can you confirm that by adding this test to the suite that it in fact passes? Test runs and passes for me. > It may not be possible to automate the testing for all platforms. Perhaps that may be the issue since I am using XP.
(In reply to comment #8) > (In reply to comment #) > > Can you confirm that by adding this test to the suite that it in fact passes? > > Test runs and passes for me. > > > It may not be possible to automate the testing for all platforms. > > Perhaps that may be the issue since I am using XP. Thanks for verifying. I'm not sure what to do with these tests then. It appears that you can't modify the Vista shell. Might be an SWT bug or a platform limitation.
I tried this test on XP and it doesn't work, even with the addition of shell.forceActive(). I don't understand how it could work for you and not for me.
Created attachment 127816 [details] shell: active & junits I created a new patch, perhaps I made a mistake in my previous one. I tested this one in a new workspace, and the tests passed. Please let me know if there are still issues. Thanks!
Thanks. 1) The patch contains changes for CTabFolder-CTabItem. I believe this is a separate bug. Did you mean to include it? (I assume no) 2) When adding new classes the copyright should say IBM. DynamicPseudoClassesSWTActiveHandler has a copy of the copyright showing Angelo as the contributor. Please correct to specify IBM. After removing the change in (1) I reran the tests and you're right it now passes on XP. I will try later on Vista. Please resubmit patch with (1) and (2) addressed.
(In reply to comment #12) > Thanks. > > 1) The patch contains changes for CTabFolder-CTabItem. I believe this is a > separate bug. Did you mean to include it? (I assume no) Sorry, my mistake, I forgot I made changes for another bug. The new patch should be fine. Thanks.
Created attachment 127839 [details] shell: active & junits
You still forgot to fix the copyrights :) I fixed it (and added one for the test), released the patch to HEAD. I'm going to test it on Vista before closing.
(In reply to comment #15) > You still forgot to fix the copyrights :) > I wans't sure if I should change the contributors or not :S I only changed the top part beside "Copyright (c) 2008" . Well as long as the patch is working now :)
(In reply to comment #16) > I wans't sure if I should change the contributors or not :S I only changed the > top part beside "Copyright (c) 2008" . Why? (e.g. because you copied code which had Angelo in the copyright?)
(In reply to comment #17) > Why? (e.g. because you copied code which had Angelo in the copyright?) > Because it says "inital API and implementation" and I figured if Angelo's name was there, and the new class uses the same API as the other ones, that it would make sense to include him. If not, I'll make sure to change it in the future.
(In reply to comment #18) > Because it says "inital API and implementation" and I figured if Angelo's name > was there, and the new class uses the same API as the other ones, that it would > make sense to include him. If not, I'll make sure to change it in the future. Ah I see. It was a good catch (good that you're thinking of such things). I think it's ok in this case - we're conforming to an API, not making a new one that's a copy of one.
Created attachment 128045 [details] shell: active & junits I was going through some junits and noticed that I forgot to remove a "System.out" that I used in the code ... this patch no longer has it.
This patch is all conflicts for me against latest source. I assume it's because your workspace is out of date vs. HEAD. Note that I *had* committed the previous work (comment #15). Any new patches should be deltas from that, from what's in HEAD.
(I can't make sense of it as is, you will need to get caught up and submit a new patch).
Created attachment 128268 [details] fix in the junit test Sorry, I forgot you committed it to HEAD. It was just a simple System.out I forgot to remove. This should fix it.
Thanks. I should've spotted it when I reviewed the patch. Released this update to HEAD>
Test passes on Vista, closing bug.
