[webkit-dev] StyleBuilder vs StyleResolver

[Antti Koivisto]

On Fri, Apr 12, 2013 at 4:50 PM, Dirk Schulze <dschulze at adobe.com> wrote:

>
> On Apr 12, 2013, at 1:08 AM, Antti Koivisto <koivisto at iki.fi> wrote:
>
> > On Fri, Apr 12, 2013 at 7:36 AM, Dirk Schulze <dschulze at 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 at lists.webkit.org
> > https://lists.webkit.org/mailman/listinfo/webkit-dev
> >
> > _______________________________________________
> > webkit-dev mailing list
> > webkit-dev at lists.webkit.org
> > https://lists.webkit.org/mailman/listinfo/webkit-dev
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-dev/attachments/20130412/bf10deed/attachment.html>