On Tue, 16 Jul 2024 19:33:10 GMT, Andy Goryachev <ango...@openjdk.org> wrote:
> I know it might be quite a bit of work, but I think it might be worth going > through an exercise to enumerate all possible scenarios in a test. > > As a reviewer, I cannot begin to figure out the possible consequences of the > change in this PR without documenting the rules first. The CSS rules, or the rules surrounding lookups? For lookups: **Before** If a style used a variable, and that variable was overridden in a stylesheet with a higher priority `StyleOrigin`, then the original style would be modified to have this higher priority as well. If there was a chain of references (ie. `-fx-fill-color` refers to `-fx-base`) then the highest priority encountered while resolving the variable would be used. It is possible that an inline style overrides a variable like `-fx-base`, which would be the highest `StyleOrigin` priority, and so the original style would get `INLINE` priority. There was a worry that this would badly interact with caching (as inline styles are supposed to be local to the `Node` and so by definition, can't be shared with any other nodes), however, such a separation in caches was already present (out of necessity) was this would apply to any inline style (like `-fx-text-fill: red`), whether or not it used a lookup. The separation is therefore always present when an inline style is present, irrespective of the ent ire variable lookup system. **After** A lookup variable does not alter the priority of the style referring the variable in any way. This both simplifies the code, as well as the reasoning about your style priorities. **Potential Consequences** First realize that this is limited in scope to that variable lookups in JavaFX allow. They only allow color replacements. Also realize that the combination of setting some colors with a setter, while adjusting others by changing style sheet variables like `-fx-base` is a rare one. Most often projects will mandate it is one or the other (ie. specify all colors in stylesheets, or don't use stylesheets at all). 1. Leaving it as-is: versions of JavaFX prior to 21 behaved differently in this matter (due to a bug), and people have noticed that in 21+ FX is now overriding color setters as if some styles are originating from higher priority stylesheets. This is limited in scope to colors, but could get worse if the system is extending to allow for other uses of variables. 2. Modifying the (undocumented) behavior to be more in line with what one would expect: due to a seemingly unrelated bug, versions before JavaFX 21 often didn't override setter values (all of them, in specific situations, not just colors). This was mostly expected as both the call to the setter and the (higher priority) author stylesheets are under control of the developer, and so if there was any conflict, it could be solved by the developer. In other words, developers could fix their own mistake, even though it may have been a bug on our side. TLDR; modifying the behavior will make JavaFX operate more like it did prior to JavaFX 21. None of them however will work exactly the same in all situations (ie. JavaFX 20 works differently from 21, and a future version where this PR merged will work differently again. However, that future version will work closer to how JavaFX behaved before version 21. Maybe a table will help... First, the order form highest to lowest priority is: `INLINE` (using `Node::setStyle`), `AUTHOR` (from user set stylesheets), `USER` (for setters) and then `USER_AGENT` (a stylesheet like Modena set by FX itself). | Action \ Version |<21|21+|With Fix| |---|---|---|---| |Setting a color|Not overridden by anything(*)|Overridden by AUTHOR and INLINE styles, and by any USER_AGENT styles if they use a variable and that variable was specified in an AUTHOR or INLINE style|Only overridden by AUTHOR or INLINE styles (as documented)| |Setting a non-color|Not overridden by anything(*)|Only overriden by AUTHOR or INLINE styles (as documented)|Same as 21+| |Using `-fx-base` in AUTHOR stylesheet, as well as some direct styles|Both USER_AGENT styles that use `-fx-base` and AUTHOR styles that apply things directly would be in competition who is the most specific|Same as before 21|USER_AGENT styles that use `-fx-base` are not in direct competition with AUTHOR styles unless more specific per CSS rules| (*) most of the time, the situation is a bit complicated due to cache sharing ------------- PR Comment: https://git.openjdk.org/jfx/pull/1503#issuecomment-2231892066
