On Fri, Apr 12, 2013 at 4:50 PM, Dirk Schulze <dschu...@adobe.com> wrote:
> > On Apr 12, 2013, at 1:08 AM, Antti Koivisto <koivi...@iki.fi> wrote: > > > On Fri, Apr 12, 2013 at 7:36 AM, Dirk Schulze <dschu...@adobe.com> > wrote: > > Hi, > > > > The style of CSS properties is either set in StyleBuilder/CSSProperty or > in StyleResolver (alias CSSStyleSelector). > > > > StyleResolver has a giant switch statement to handle all CSS property > values and set the style. It is the historical way to build the style. > > > > StyleBuilder was introduced ~2 years ago. Instead of a giant switch to > handle all property styles, it has a concept of template to combine CSS > property handling. > > > > In these last two years new properties were mainly added to > StyleBuilder, older properties were left alone in StyleResolver. The > concept of StyleBuilder was always controversial[1][2]. A lot of people had > concerns that StyleBuilder is less readable and makes it harder to > understand the code. > > > > I personally am more worried that we still have two ways to set the > style. I think it is bad to keep half of the properties in StyleResolver > and the other half in StyleBuilder. We may use the general "spring cleanup" > to revalidate the concept of StyleBuilder and StyleResolver and decide to > use the one or the other concept. > > > > Any thoughts? > > > > Fully agreed. I'm still sad I couldn't stop this refactoring initially. > So much wasted effort. > > > > Having property applying code in a separate class instead of piling it > back to StyleResolver still makes sense though and StyleBuilder is a good > name. Maybe something like this would be a good strategy? > > > > rename StyleBuilder -> DeprecatedStyleBuilder > > create new StyleBuilder > > move the giant switch and the related functions from StyleResolver to > StyleBuilder > > move individual properties from DeprecatedStyleBuilder to StyleBuilder > until nothing remains > > delete DeprecatedStyleBuilder > > I agree that StyleBuilder is the better name. Do you really want to have > two huge renamings? Can't we just move everything over to StyleResolver and > then rename it to StyleBuilder? After all, we just removed one build system > yet ;) However, I am fine with both. > StyleBuilder is an implementation detail of StyleResolver and is not referenced from anywhere else. I don't changing its name qualifies as a "huge renaming". The transition may take a while. I like strategy like this as it documents both the current state and the end goal. antti > I would definitely help to make this possible but would appreciate to get > help from others as well. > > Here is a master bug for this change: > https://bugs.webkit.org/show_bug.cgi?id=114508 > > Greetings, > Dirk > > > > > > > > antti > > > > > > Greetings, > > Dirk > > > > [1] https://bugs.webkit.org/show_bug.cgi?id=54707 > > [2] https://bugs.webkit.org/show_bug.cgi?id=102844 > > _______________________________________________ > > webkit-dev mailing list > > webkit-dev@lists.webkit.org > > https://lists.webkit.org/mailman/listinfo/webkit-dev > > > > _______________________________________________ > > webkit-dev mailing list > > webkit-dev@lists.webkit.org > > https://lists.webkit.org/mailman/listinfo/webkit-dev > >
_______________________________________________ webkit-dev mailing list webkit-dev@lists.webkit.org https://lists.webkit.org/mailman/listinfo/webkit-dev