Community
Participate
Working Groups
See bug #278479. It appears that engine.reset() isn't clearing everything up. For example, in LabelTest with the test: public void testAlignment() throws Exception { Label labelToTest = createTestLabel("Label { alignment: right }"); assertEquals(SWT.RIGHT, labelToTest.getAlignment()); clearAndApply(engine, labelToTest, "Label { alignment: left; }"); assertEquals(SWT.LEFT, labelToTest.getAlignment()); clearAndApply(engine, labelToTest, "Label { alignment: center; }"); assertEquals(SWT.CENTER, labelToTest.getAlignment()); } You will always fail on the last clearAndApply(), even if you swap the last two tests (left vs. center). The reason the exception is occuring is because we end up somehow having a property 'background' with value the string "null".
Kai, I notice you're relying on this for your contacts demo.
Interesting that ShellActiveTest is also failing in the same way ("This isn't an RGBCOlor type): Shell shell = createShell("Shell:active {background-color: #FF0000;}\n" + "Shell {background-color: #0000FF;}"); assertEquals(RED, shell.getBackground().getRGB()); Shell newShell = createShell("Shell { background-color: #0000FF; }"); assertEquals(BLUE, shell.getBackground().getRGB()); so it's not just use of reset(). It's quite possible that I introduce this bug through changes in Measure although I suspect I might've just exposed a bug elsewhere.
It's very very odd that only tests with *two reuses* of the same engine were failing (ie. three groups of parse/test).
The swt tests passed on http://download.eclipse.org/e4/downloads/drops/I20090521-1930/results/html/org.eclipse.e4.ui.tests.css.swt_linux.gtk.ppc.html
Calling elementsContext.clear() at the end of the reset() method seems to resolve the font problem uncovered by bug 271490.
(In reply to comment #5) > Calling elementsContext.clear() at the end of the reset() method seems to > resolve the font problem uncovered by bug 271490. Somewhat but introduces other problems. In the editor example, what happens with that change is that if you delete an expression, we're not resetting the widget back to its default state. What I'm seeing now is in the editor example if you type: In the example: CTabFolder { color: darkblue; background-color: #D0D0E0; } if you delete the two middle lines, so make it: CTabFolder { } You won't see any change. I believe the CSSElementContext eventually holds onto the captured widget 'default' state. doing a clear() throws out all that information, since removing an expression does not revert the widget state. However, this is likely a pointer to the real bug, which is that we're not correctly capturing the font state for CTabFolder v.s. CTabItem...
The test case below will show the problem. It seems setFont(Font) is being called on CTIs even though the stylesheet only defines something for the CTF. For me, it fails in the during size 16 but never on size 14. Display display = Display.getCurrent(); CSSEngine engine = createEngine("CTabFolder { font-size: 12 }", display); engine.applyStyles(shell, true); assertEquals(12, getFontHeight(folder)); CTabItem[] items = folder.getItems(); for (int i = 0; i < items.length; i++) { assertEquals(12, getFontHeight(items[i])); } clearAndApply(engine, shell, "CTabFolder { font-size: 14 }"); assertEquals(14, getFontHeight(folder)); items = folder.getItems(); for (int i = 0; i < items.length; i++) { assertEquals(14, getFontHeight(items[i])); } clearAndApply(engine, shell, "CTabFolder { font-size: 16 }"); assertEquals(16, getFontHeight(folder)); items = folder.getItems(); for (int i = 0; i < items.length; i++) { assertEquals(16, getFontHeight(items[i])); }
Here's a trace I generated. Thread [main] (Suspended (breakpoint at line 944 in CTabItem)) CTabItem.setFont(Font) line: 944 CSSPropertyFontSWTHandler.setFont(Widget, Font) line: 34 CSSPropertyFontSWTHandler.onAllCSSPropertiesApplyed(Object, CSSEngine) line: 124 CSSSWTEngineImpl(AbstractCSSEngine).applyStyleDeclaration(Object, CSSStyleDeclaration, String) line: 440 CSSSWTEngineImpl(AbstractCSSEngine).applyDefaultStyleDeclaration(Object, boolean, CSSStyleDeclaration, String) line: 584 CSSSWTEngineImpl(AbstractCSSEngine).applyStyles(Object, boolean, boolean) line: 316 CSSSWTEngineImpl(AbstractCSSEngine).applyStyles(Object, boolean) line: 293 CSSSWTEngineImpl(AbstractCSSEngine).applyStyles(Object, boolean, boolean) line: 365 CSSSWTEngineImpl(AbstractCSSEngine).applyStyles(Object, boolean) line: 293 CSSSWTEngineImpl(AbstractCSSEngine).applyStyles(Object, boolean, boolean) line: 365 CSSSWTEngineImplTest(CSSSWTTestCase).clearAndApply(CSSEngine, Widget, String) line: 62 CSSSWTEngineImplTest.testReset() line: 61
Good digging. Interesting, if put your test into a junit I don't get the error you describe, I get the error in comment #2. But it's the two clearAndApply's. If I change clearAndApply() to do: engine.applyStyles(widget, true, *false*); ie. don't capture widget state, my failing test now passes. The captured data is wrong. This is some trace info for the new test: ***Applying style: CTabFolder { font-size: 12 } Setting font size widget: CTabFolder {} size: 12 Setting font size widget: CTabFolder {} size: 12 ***Clear and applying style: CTabFolder { font-size: 14 } Capturing font size widget: CTabFolder {} size: 12 Capturing font size widget: CTabFolder {} size: 12 Setting font size widget: CTabFolder {} size: 14 Setting font size widget: CTabFolder {} size: 14 Capturing font size widget: CTabItem {TAB 1} size: 14 <<==WRONG! Capturing font size widget: CTabItem {TAB 2} size: 14 <<==WRONG! ***Clear and applying style: CTabFolder { font-size: 16 } So we're capturing immediately after setting, and capturing the new values that we just set.
(In reply to comment #9) > Good digging. > > Interesting, if put your test into a junit I don't get the error you describe, > I get the error in comment #2. But it's the two clearAndApply's. If I change > clearAndApply() to do: > > engine.applyStyles(widget, true, *false*); > > ie. don't capture widget state, my failing test now passes. The captured data > is wrong. > > This is some trace info for the new test: > > ***Applying style: CTabFolder { font-size: 12 } > Setting font size widget: CTabFolder {} size: 12 > Setting font size widget: CTabFolder {} size: 12 > ***Clear and applying style: CTabFolder { font-size: 14 } > Capturing font size widget: CTabFolder {} size: 12 > Capturing font size widget: CTabFolder {} size: 12 > Setting font size widget: CTabFolder {} size: 14 > Setting font size widget: CTabFolder {} size: 14 > Capturing font size widget: CTabItem {TAB 1} size: 14 <<==WRONG! > Capturing font size widget: CTabItem {TAB 2} size: 14 <<==WRONG! > ***Clear and applying style: CTabFolder { font-size: 16 } I think the problem is that all widget state must be captured recursively prior to setting a value. Instead, what happens is that each widget captures/sets, then recurses on its children. But setting the font on the CTF sets it on the CTI, and when we go to capture on the child CTI its state has already been changed. So this recursive approach doesn't work where there are side effects on children when setting on the parent. Still to understand: 1) Why are we setting/capturing twice in the trace above? 2) Bad values being captured (comment #1, comment #2).
(In reply to comment #10) > 1) Why are we setting/capturing twice in the trace above? Assuming we're tracing the same thing (putting the sysout statements in the same place), the "setting" bit seems to be caused by the "selected" pseudo. You can test this by just commenting out the implementation of the computeStaticPseudoInstances() method in SWTElement.
CSSPropertyHandlerSimpleProviderImpl.getDefaultCSSStyleDeclaration() attempts to retrieve the default values for *all* properties (getAllCSSPropertyNames()), even ones we're not setting. This is the attempt to capture the initial widget state. But I don't understand how this can work in practice since many of the property getters will return null, which happily becomes the string null, which later we will attempt to set back into the widget to restore it to its default state. We would need special code in all the property setters to detect the null as a way to say "revert the property to its factory default". But we don't do this everywhere. I'm not sure this is the right approach anyway. For one, we'd end up resetting every property of every widget, which seems overkill. What we should be doing instead (assuming we still want to capture default state) is: 1) Retrieve property values for all properties we're _about_ to change. 2) Handle null as a property value being set and interpret it correctly.
(In reply to comment #11) > (In reply to comment #10) > > 1) Why are we setting/capturing twice in the trace above? > > Assuming we're tracing the same thing (putting the sysout statements in the > same place), the "setting" bit seems to be caused by the "selected" pseudo. You > can test this by just commenting out the implementation of the > computeStaticPseudoInstances() method in SWTElement. I'm sure you're right thanks. I've logged bug #281284 wrt my comment #8 about ordering of capture vs. apply. I've logged bug #281286 wrt my comment #12 about that we shouldn't try to capture *all* values. Special note should be made of values like "background" (the one failing in this example) which is a composite property and therefore never returns a value for retrieval (instead you capture "background-color:", etc.)
I don't think there's anything I can touch in here before 0.9, removing milestone.
Kevin doesn't work on e4 anymore.
This bug hasn't had any activity in quite some time. Maybe the problem got resolved, was a duplicate of something else, or became less pressing for some reason - or maybe it's still relevant but just hasn't been looked at yet. If you have further information on the current state of the bug, please add it. The information can be, for example, that the problem still occurs, that you still want the feature, that more information is needed, or that the bug is (for whatever reason) no longer relevant. -- The automated Eclipse Genie.
