Bugzilla – Full Text Bug Listing
| Summary: | [CSS] Cascading policy used to apply rules is broken | ||||||
|---|---|---|---|---|---|---|---|
| Product: | [Eclipse Project] Platform | Reporter: | Andrea Guarinoni <andrea.guarinoni> | ||||
| Component: | UI | Assignee: | Paul Webster <pwebster> | ||||
| Status: | VERIFIED FIXED | QA Contact: | Daniel Rolka <daniel.rolka> | ||||
| Severity: | normal | ||||||
| Priority: | P3 | CC: | andrea.guarinoni, azerr, daniel.rolka, Lars.Vogel, stefan, thomas | ||||
| Version: | 4.4 | ||||||
| Target Milestone: | 4.4 M7 | ||||||
| Hardware: | PC | ||||||
| OS: | All | ||||||
| Whiteboard: | |||||||
| Bug Depends on: | |||||||
| Bug Blocks: | 260408, 552476 | ||||||
| Attachments: |
|
||||||
Although we are not 100% compatible with the Web CSS support, I agree that we should take a look at it Daniel (In reply to Daniel Rolka from comment #1) > Although we are not 100% compatible with the Web CSS support, I agree that > we should take a look at it This bug makes the dark theme hard to maintain and unnecessary complex. Andrea and I can try to look at this one. Can you give us some pointer where to start? Hi,
At first I would like take the given sample with HTML. If you write like you have explained :
-----------------------------------------
body, body > *, body > * > * {
background-color: red;
margin: 50px;
padding: 50px;
}
panel{
background-color: blue;
}
-----------------------------------------
panel is blue.
But if change the order of the style declaration :
-----------------------------------------
panel{
background-color: blue;
}
body, body > *, body > * > * {
background-color: red;
margin: 50px;
padding: 50px;
}
-----------------------------------------
panel is red.
It means that the choose of the style belongs to :
* the specificity : "body > * > *" have the same priority (in css context, it's called "specificity") than "panel"
* the position
So it seems for E4, it's the same problem. Have you tried to change the order of the style declaration?
To understand the problem, I sugest you to set a breakpoint in a ViewCSSImpl#getComputedStyle and see
-----------------------------------------
int specificity = extendedSelector.getSpecificity();
-----------------------------------------
I suggest you to add JUnit with your case in https://eclipse.googlesource.com/platform/eclipse.platform.ui/+/I20130515-2000/tests/org.eclipse.e4.ui.tests.css.core/src/org/eclipse/e4/ui/tests/css/core/parser/ViewCSSTest.java
by modyfing TestElement to pass the parentNode.
Hope my answer will help you.
Regards Angelo
(In reply to Angelo ZERR from comment #3) Thanks Angelo for the reply. @Andrea, can you please test if the result changes if you change the order of declaration? Created attachment 241528 [details]
TestCase
After some hours of stepping through the CSS engine and trying out different things, I have to admit that I don't see an error. I also cannot reproduce the initial example of Andrea.
I have attached my test cases to this bug. The class can be copied into /org.eclipse.e4.ui.tests.css.swt/src/org/eclipse/e4/ui/tests/css/swt and added to the test suite org.eclipse.e4.ui.tests.css.swt.CssSwtTestSuite
For me all the tests pass.
In particular, the initial example works for me.
From debugging, I can tell that
'Shell > * > * {...}' and 'ToolBar {...}' actually lead to the same specificity (1), because one element is named in the selector.
When computing the resulting style, the css rules are first ordered by specificity (ascending) and then by order of encountering them (top to bottom). Every rule that comes after another rule in that ordering, potentially cascades the one before (if they give the same properties, that is).
Therefore, as stated before 'Shell > * > * {...}' will be cascaded by a subsequent 'ToolBar {...}'. BUT a rule 'Shell > Composite > * {...}' will not be cascaded by 'ToolBar {...}', because 'Shell > Composite > * {...}' has a specificity of 2 (because the selector contains two concrete elements).
Please review the attached test case. Maybe I missed a point somewhere, but to me, it looks like the CSS engine works as specified.
(Of course, if the engine works as intended, the next step would be to analyze the dark theme CSS files and check whether they can be simplified in some way to improve maintainability).
Thanks Stefan for the analysis! Looks to me that you getting really into the CSS engine, thanks a bunch for that. (In reply to Stefan Winkler from comment #5) > Created attachment 241528 [details] > TestCase Can you convert that into Gerrit? > Of course, if the engine works as intended, Looks like it, can you please open a new bug to simplify the CSS? I think we had also a bug from Andrea, in which he reported that a value from an imported style sheet cannot be overridden by another CSS file. That lead to a LOT of duplication in the CSS files, maybe you can also look at that? (In reply to Lars Vogel from comment #6) > Can you convert that into Gerrit? here it is: https://git.eclipse.org/r/24358 There are also three additional test cases for the CSS core tests. I have included them as well. > Looks like it, can you please open a new bug to simplify the CSS? I have opened Bug 431845 for that. > I think we had also a bug from Andrea, in which he reported that a value > from an imported style sheet cannot be overridden by another CSS file. That > lead to a LOT of duplication in the CSS files, maybe you can also look at > that? This seems to be Bug 430052. I'll have a look tomorrow. Released the test cases. Thanks Stefan. http://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/?id=bbcd42522c3f7fb76f0ec98666ef20786297a58e PW In 4.4.0.I20140428-2000 PW |
There is a big anomaly about how CSS rules are applied that go against one basic CSS policy. Currently, when a broader rule is set by using the global selector (*), it cannot be always overriden by a following more specific rule. See the case: Shell, Shell > *, Shell > * > * { background-color: red; } ToolBar { background-color: blue; } What is expected is that any ToolBar will be drawn with a blue background-color since the rule should have an higher weight because it's following the other. What happens is that the rule is ignored for all those ToolBar that are catched by a Shell > ToolBar and Shell > * > ToolBar selectors. As a comparison, a similar real CSS code in a browser work as expected: <html> <head> <style type="text/css"> body, body > *, body > * > * { background-color: red; margin: 50px; padding: 50px; } panel{ background-color: blue; } </style> </head> <body> <div> <panel></panel> </div> </body> </html> Notice that the only way to set a style for a widget that has not a CSS id, a CSS class and a seekable Widget name is using the global selector (*) and then 'fine-tune' the seekable elements by adding following rules for them in the stylesheet. Currently, to only way to achieve the desired result is to add a lot of dirty rules that address all the generated implicit cases: eg. Shell, Shell > *, Shell > * > * { background-color: red; } ToolBar, Shell > ToolBar, Shell > * > ToolBar { background-color: blue; } But this isn't right behavior.