API REVIEW: move com.sun.javafx.css.StyleManager to javafx.scene.StyleManager

[David Grieve]

1. There is no impact on any future API to support programmatic CSS access (http://javafx-jira.kenai.com/browse/RT-17293). 

2. StyleManager should be a final variable in Scene. Even if more of the css implementation were to be moved to javafx.scene (something Richard is arguing for), the getter would still need to be public since StyleManager is used outside of javafx.scene (in javafx.scene.controls and possibly by SceneBuilder)

3. I didn't notice the cut-n-paste error. The two methods should be setDefaultUserAgentStylesheet and addUserAgentStylesheet. Both are not strictly required. addUserAgentStylesheet could be enough. However, normal declarations from the default user agent stylesheet need be first in the cascade so they may be overwritten by declarations from other user agent stylesheets. Assuming the first UA stylesheet is meant to be the default is not a valid assumption. The list of UA stylesheet URL strings could be exposed, but then anyone could come along and insert their UA stylesheet and make it the default. Maybe that isn't a bad thing, but it is a change in how the system behaves.

The two methods allow UA stylesheets to be added even before the default is set. It isn't fool proof. There would be nothing to prevent someone from calling setDefault UserAgentStylesheet before controls calls it to set caspian. Again, maybe this isn't a bad thing. 

On Jul 25, 2012, at 4:05 PM, Jonathan Giles wrote:

> Personally I have no issue with doing this (StyleManager on Scene and Parent), but then I do not own these two classes. I do have a few questions though:
> 
> 1) How does this impact any future APIs to support programmatic CSS access (i.e. Java API to get/set/observe CSS property values)?
> 
> 2) In general we try not to have getters/setters when the object is not being exposed as a full property - but I guess it makes no sense to allow people to set or observe changes to the StyleManager field?
> 
> 3) I know I could dive into the code, but what is the difference between setDefaultUserAgentStylesheet and addDefaultUserAgentStylesheet? Are both necessary?
> 
> -- Jonathan
> 
> On 26/07/2012 5:15 a.m., David Grieve wrote:
>> Background:
>> This request is related to http://javafx-jira.kenai.com/browse/RT-23460. StyleManager's job it is to keep track of stylesheets, manage the cascade, and create StyleHelpers for nodes. StyleHelper is the code that applies css styles to nodes.
>> 
>> Currently, there StyleManager is a singleton but it can manage stylesheets from many different scenes. To do this, it maintains a table that maps Scene to a StylesheetContainer which contains the list of declarations from all the stylesheets associated with the scene. RT-23460 seeks to eliminate the lookup of Scene to StylesheetContainer. One such way to do this is for Scene to own its StylesheetContainer, as is proposed in the bug report. Another way is to have Scene own its own StyleManager. This latter approach is preferred (well, by me at least) since it greatly reduces the complexity of StyleManager and makes other optimizations possible.
>> 
>> Proposed changes:
>> 
>> I propose adding a StyleManager member to Scene and moving StyleManager to the javafx.scene package as an abstract class with the minimum required public API.
>> 
>> Scene would add:
>>     private final StyleManger styleManager;
>>     public final StyleManager getStyleManager()
>> 
>> Because Scene would have public API that returns a StyleManager, StyleManager itself needs to be public API and would need to be moved from com.sun.javafx.css to javafx.scene
>>  I propose StyleManager's public API as:
>> public abstract StyleHelper getStyleHelper(Node node)
>> public final ObservableList<String> getStylesheets() -- the list of stylesheet URL strings for the Scene. Scene's getStylesheets() method would simply return styleManager.getStylesheets()
>> public void setDefaultUserAgentStylesheet(String url)
>> public final void addDefaultUserAgentStylesheet(String url) -- support Controls that have their own UA stylesheet, plus adding UA stylesheets for embedded
>> public final List<String> getUserAgentStylesheets() - return immutable list of UA stylesheet URL strings
>> Note that StyleManager returns a StyleHelper. Hence, StyleHelper would also need to be part of the public API. There are only two methods in StyleManager that need to be exposed
>> public abstract void transitionToState() - this method is called from Node when css needs to be applied
>> public abstract boolean isPseudoclassUsed(String pseudoclass) - this method is also called from node to determine whether or not the node's pseudo-class state would cause css to be applied
>> 
>> Note that neither the proposed StyleManager API nor the StyleHelper API pull in any other API not already part of the public API.
>> 
>> Having Scene own a StyleManager also has the benefit of being able to avoid looking up the parent chain to see if the scene's window is a popup or not. That logic can be moved to the constructor. With this change, StyleManager can be further streamlined by eliminating the StylesheetContainer (StylesheetContainer gets moved into the body of the StyleManagerImpl) and the defaultContainer.
>> 
>> Further, since a Parent can also have stylesheets, I also propose adding StyleManager to Parent with the same API as in Scene.
>>     private final StyleManger styleManager;
>>     public final StyleManager getStyleManager()
>> 
>> This will help eliminate the parent stylesheet lookup that currently happens in StylesheetContainer.
>> 
>> 
>> David Grieve | Principal Member of Technical Staff
>> Mobile: +16033121013
>> Oracle Java Client UI and Tools
>> Durham, NH 03824
>>  Oracle is committed to developing practices and products that help protect the environment
>> 
> 


David Grieve | Principal Member of Technical Staff
Mobile: +16033121013 
Oracle Java Client UI and Tools
Durham, NH 03824 
 Oracle is committed to developing practices and products that help protect the environment