[External] : Re: CSS Lookups and their origins (possible regression)
Kevin Rushforth
kevin.rushforth at oracle.com
Tue Jul 9 20:16:50 UTC 2024
My reading of this is that the fix for the caching bug, JDK-8245919 [1]
via PR #1072 [2], has inadvertently exposed previously hidden behavior
that is at best an undesirable feature and at worst a bug (I'd call it a
bug). It seems quite unexpected to me that overriding a variable defined
in a user agent stylesheet in an AUTHOR stylesheet would elevate all
properties derived from that variable to AUTHOR stylesheet status.
Assuming I'm not missing something, we ought to consider fixing this.
-- Kevin
[1] https://bugs.openjdk.org/browse/JDK-8245919
[2] https://github.com/openjdk/jfx/pull/1072
On 7/9/2024 11:21 AM, John Hendrikx wrote:
>
> Hi Andy,
>
> I'm confused, nowhere do I propose to remove or otherwise make the CSS
> reference system implemented by FX unusable.
>
> I'm first trying to ascertain if this would be expected behavior (it
> is indeed unspecified, and the default currently seems to have been
> chosen for implementation ease, not for user friendliness).
>
> **IF** we're considering this worth changing, the change would simply
> be that when you override a variable (like -fx-base) that this is done
> WITHOUT elevating all styles that use it to the level of an AUTHOR
> stylesheet (ie. they remain at USER_AGENT level as they're specified
> by Modena). This is not a bad view, because in a sense, we're not
> really specifying a style, we're only overriding a variable. The
> actual style is still specified in Modena, which is a USER_AGENT level
> stylesheet.
>
> As for the bug fix, please read up a bit more on what was fixed, and
> what this is now exposing. The fix is almost completely unrelated (it
> fixed accidental changes to unrelated controls at the same level (ie.
> siblings) due to cache sharing where one has had a programmatic
> change, and the other didn't). This was caused by a CSS calculation
> bug (calculation was skipped for all styleable properties that already
> had a setter change, if they were encountered first by the CSS
> system). Now that this isn't the case anymore, set values are
> overwritten with CSS styles more aggressively. Normally those however
> are only styles that originate from an AUTHOR stylesheet, so this can
> be seen as expected by the user (after all, they WROTE that
> stylesheet). But because all styles that use a variable are being
> promoted to AUTHOR level, this also includes all unseen styles in
> Modena if you specify the variable in your AUTHOR stylesheet.
>
> > Nowhere did we **actualy** override -fx-text-fill " is not
> technically correct since this color depends on -fx-base.
>
> That really depends on your view point. Is overriding a variable the
> same as defining all styles (in your AUTHOR stylesheet) that use that
> variable? If it was a pre-processor, that created a fully resolved
> Modena.css, then this would not be the case. But it is not
> implemented as such.
>
> > And I would not want to change how it works currently because this
> is the only way (short of overwriting the whole modena.css
> styleshseet) for an application to effect a system-wide change like
> reacting to changes in the user preferences or the platform theme.
>
> To be clear, I'm not proposing to change that at all.
>
> --John
>
> On 09/07/2024 20:00, Andy Goryachev wrote:
>>
>> 1) a buggy implementation coupled with lack of specification creates
>> a certain expectation
>>
>> 2) bug gets fixed
>>
>> 3) people complain because the feature now works as it should?
>>
>> I think (and this is my personal opinion, in the absence of a formal
>> specification) that this works as expected now. The statement "
>> Nowhere did we **actualy** override -fx-text-fill" is not technically
>> correct since this color depends on -fx-base.
>>
>> And I would not want to change how it works currently because this is
>> the only way (short of overwriting the whole modena.css styleshseet)
>> for an application to effect a system-wide change like reacting to
>> changes in the user preferences or the platform theme.
>>
>> -andy
>>
>> *From: *John Hendrikx <john.hendrikx at gmail.com>
>> *Date: *Tuesday, July 9, 2024 at 10:45
>> *To: *Andy Goryachev <andy.goryachev at oracle.com>, openjfx-dev
>> <openjfx-dev at openjdk.org>
>> *Subject: *Re: [External] : Re: CSS Lookups and their origins
>> (possible regression)
>>
>> Well, it is coming as a surprise to many. With the fix for the CSS
>> caching bug since JavaFX 21, this "normal" behavior is becoming much
>> more obvious.
>>
>> Let me repeat one more time:
>>
>> If I have a Label, and I manually set its text fill with a setter to
>> YELLOW. In JavaFX 17, when I now add a stylesheet that is empty aside
>> from `-fx-base: WHITE`, the label's text fill stays YELLOW.
>>
>> Now do this in JavaFX 21. As soon as you add the stylesheet with
>> `-fx-base: WHITE` in it, the set value to YELLOW is overridden, even
>> though technically this value for -fx-text-fill is defined by Modena
>> (which should not be overriding set values). Nowhere did we
>> **actualy** override -fx-text-fill, yet the CSS subsystem now sees
>> **all** values defined by Modena that are somehow linked to -fx-base
>> as defined directly by the developer...
>>
>> The reason this didn't happen in JavaFX prior to 21 is because there
>> was a bug where a CSS value was not fully calculated if the property
>> it encountered was overridden via a set value. That was a bug however
>> as cache entries are shared amongst similar styled nodes, and so not
>> calculating it fully could have effects on other nodes that shared
>> that cache entry but did NOT have a property set directly. Now that
>> this bug is fixed, this problem is odd behavior is popping up where
>> simply specifying -fx-base in an empty stylesheet is somehow
>> overriding a programmatically set text fill. Users are confused by
>> this, as nowhere in their stylesheet do they themselves override text
>> fill.
>>
>> This entire mechanism is not specified by CSS, but is unique to FX.
>> The most similar mechanism in CSS (see Michael's answer) says the
>> priority of a style should not be changed when it is using a reference.
>>
>> --John
>>
>> On 09/07/2024 17:43, Andy Goryachev wrote:
>>
>> > all styles used in Modena that rely on -fx-base directly or
>> indirectly suddenly have a higher priority
>>
>> I think it works as designed (and as expected).
>>
>> -andy
>>
>> *From: *John Hendrikx <john.hendrikx at gmail.com>
>> <mailto:john.hendrikx at gmail.com>
>> *Date: *Tuesday, July 9, 2024 at 08:25
>> *To: *Andy Goryachev <andy.goryachev at oracle.com>
>> <mailto:andy.goryachev at oracle.com>, openjfx-dev
>> <openjfx-dev at openjdk.org> <mailto:openjfx-dev at openjdk.org>
>> *Subject: *[External] : Re: CSS Lookups and their origins
>> (possible regression)
>>
>> It's not that you can't use -fx-base, but that as it is currently
>> that all styles used in Modena that rely on -fx-base directly or
>> indirectly suddenly have a higher priority (above setters) even
>> though you didn't specifically specify them in your own
>> stylesheet. All such styles are being elevated from USER_AGENT
>> to AUTHOR level (which is above USER level which is used for
>> setters).
>>
>> --John
>>
>> On 09/07/2024 17:03, Andy Goryachev wrote:
>>
>> I've used this feature in the past to change the colors in
>> all the controls, so to me this is the expected behavior.
>>
>> So in your case (if I got it right), you need to set the
>> direct style on the label (.setStyle("-fx-text-fill:yellow"))
>> instead of setting the text fill programmatically. Right?
>>
>> -andy
>>
>> *From: *openjfx-dev <openjfx-dev-retn at openjdk.org>
>> <mailto:openjfx-dev-retn at openjdk.org> on behalf of John
>> Hendrikx <john.hendrikx at gmail.com>
>> <mailto:john.hendrikx at gmail.com>
>> *Date: *Monday, July 8, 2024 at 17:11
>> *To: *openjfx-dev <openjfx-dev at openjdk.org>
>> <mailto:openjfx-dev at openjdk.org>
>> *Subject: *Re: CSS Lookups and their origins (possible
>> regression)
>>
>> I realized I worded the TLDR poorly.
>>
>> Let me try again:
>>
>> TLDR; should styles which use references (like -fx-base used
>> in Modena) become AUTHOR level styles if -fx-base is
>> specified in an AUTHOR stylesheet? The act of simply
>> specifying -fx-base in your own AUTHOR stylesheet elevates
>> hundreds of styles from Modena to AUTHOR level, as if you
>> specified them directly...
>>
>> --John
>>
>> On 09/07/2024 02:07, John Hendrikx wrote:
>>
>> Hi List,
>>
>> TLDR; should a CSS reference like -fx-base convert all
>> styles that use this value (or derive from it) become
>> AUTHOR level styles (higher priority than setters) ?
>>
>> Long version:
>>
>> In JavaFX 21, I did a fix (see #1072) to solve a problem
>> where a CSS value could be reset on an unrelated control.
>>
>> This happened when the CSS engine encountered a stylable
>> that is overridden by the user (with a setter), and
>> decided NOT to proceed with the full CSS value
>> calculation (as it could not override the user setting if
>> that CSS value had lower priority). However, not
>> proceeding with the calculation meant that a "SKIP" was
>> stored in a shared cache which was incorrect. This is
>> because when this "SKIP" is later encountered for an
>> unrelated control (the cache entries are shared for
>> controls with the same styles at the same level), they
>> could get their values reset because they were assumed to
>> be unstyled.
>>
>> However, this fix has exposed what seems to be a deeper
>> bug or perhaps an unfortunate default:
>>
>> JavaFX has a special feature where you can refer to
>> certain other styles by using a reference (which is
>> resolved, recursively, to a final value). This does not
>> seem to be a CSS standard, but is a feature only FX has.
>>
>> It works by saying something like:
>>
>> -fx-base: RED;
>>
>> And then using it like this:
>>
>> -fx-text-fill: -fx-base;
>>
>> This feature works accross stylesheets of different
>> origins, so an AUTHOR stylesheet can specify -fx-base,
>> and when a USER_AGENT refers to -fx-base, the value comes
>> from the AUTHOR stylesheet.
>>
>> JavaFX then changes the origin of the style to the
>> highest priority encountered while resolving the
>> reference. This means that Modena can specify
>> "-fx-text-fill: -fx-base", and when "-fx-base" is then
>> part of the AUTHOR style sheet, that ALL Modena styles
>> that use -fx-base will be considered AUTHOR level styles,
>> as per this comment:
>>
>> // The origin of this parsed value is the greatest of
>>
>> // any of the resolved reference. If a resolved reference
>>
>> // comes from an inline style, for example, then the value
>>
>> // calculated from the resolved lookup should have inline
>>
>> // as its origin. Otherwise, an inline style could be
>>
>> // stored in shared cache.
>>
>> I feel that this is a really unfortunate choice. The
>> style after all was specified by Modena, only its value
>> came from another (higher priority) style sheet. I think
>> a more logical choice would have been to not change the
>> priority at all, unless a "-fx-text-fill" is explicitly
>> made part of the AUTHOR stylesheet.
>>
>> A consequence of this (and which is much more visible
>> after the fix) is that creating a Label with a
>> setTextFill(Color.YELLOW) in its constructor will only
>> result in a yellow text fill if the AUTHOR stylesheet did
>> not override any of the Modena colors involved in
>> calculating the Modena -fx-text-fill default. Overriding
>> -fx-base in any way will result in the text fill of the
>> label to be overridden (as the reference came from an
>> AUTHOR stylesheet, which trumps a setter which is of a
>> lower style origin).
>>
>> The comment also alludes to a potential problem. If an
>> inline style would specify "-fx-base", but would be
>> treated as if it came from Modena (USER_AGENT level),
>> then this value could get stored in the cache as
>> everything except INLINE styles can be cached. However,
>> I feel that the changing of style origin level was then
>> probably done to solve a CACHING problem, instead of what
>> made logical sense for users. If we agree that a
>> resolved reference should not change the style origin
>> level, then this would need to be addressed, by perhaps
>> marking such a calculated value as uncacheable, instead
>> of overloading the meaning of style origin.
>>
>> I'd like to hear your thoughts, and also how to proceed.
>> JavaFX versions before 21 seemingly allowed overriding
>> reference without much consequence because if the user
>> overrode the value manually, the cache entry would be set
>> to "SKIP". Now that this is no longer the case, JavaFX
>> more aggressively overrides user set values if they
>> happen to use a referenced value. See code below.
>>
>> --John
>>
>> .root {
>>
>> -fx-base: #ff0000;
>>
>> }
>>
>> *package*app;
>>
>> *import*javafx.application.Application;
>>
>> *import*javafx.scene.Scene;
>>
>> *import*javafx.scene.control.Label;
>>
>> *import*javafx.scene.paint.Color;
>>
>> *import*javafx.stage.Stage;
>>
>> *public**class*TestApp *extends*Application {
>>
>> *public**static**void*main(String[] args) {
>>
>> /launch/(args);
>>
>> }
>>
>> @Override
>>
>> *public**void*start(Stage primaryStage) {
>>
>> Scene scene = *new*Scene(*new*MyLabel());
>>
>> // See the difference with/without -fx-base in the
>> _stylesheet_
>>
>> scene.getStylesheets().add(TestApp.*class*.getResource("/style.css").toExternalForm());
>>
>> primaryStage.setScene(scene);
>>
>> primaryStage.show();
>>
>> }
>>
>> }
>>
>> *class*MyLabel *extends*Label {
>>
>> *public*MyLabel() {
>>
>> setTextFill(Color.YELLOW);
>>
>> setText("Hello world");
>>
>> }
>>
>> }
>>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.org/pipermail/openjfx-dev/attachments/20240709/034963ac/attachment-0001.htm>
More information about the openjfx-dev
mailing list