Bug 79939 – [WIP] Implement CSS Hierarchies

• Component: CSS
• Status: RESOLVED LATER

[Shane Stephens]

[WIP] Implement CSS Hierarchies

[Shane Stephens]

Created attachment 129509 [details]
Patch

[Shane Stephens]

This CSS feature is proposed in the following Editor's Draft: http://dev.w3.org/csswg/css3-hierarchies/, and has been discussed by the CSSWG (the group is largely favorable to the idea). It's currently a WIP. I'd appreciate feedback on whether this is the correct approach for a feature like this.

[Luke Macpherson]

Comment on attachment 129509 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=129509&action=review

> Source/WebCore/css/CSSParser.cpp:8833
> +        rule->styleRule()->adoptSelectorVector(*selectors, m_selectorVectorStack);

perhaps m_nestedSelectorStack would be a good alternative name.

> Source/WebCore/css/CSSParser.h:268
> +    Vector<OwnPtr<CSSParserSelector> >* reusableSelectorVector() { return m_reusableSelectorVector; }

resusableSelectorVector -> selectorVector (no longer re-usable).

> Source/WebCore/css/CSSParser.h:270
> +    void pushSelectorVector(bool trailingSpace)

pushNestedSelector()

> Source/WebCore/css/CSSParser.h:283
> +    void popSelectorVector()

popNestedSelector()

> Source/WebCore/css/CSSParser.h:455
> +    Vector<OwnPtr<CSSParserSelector> > *m_reusableSelectorVector;

OwnPtr<Vector<OwnPtr<CSSParserSelector> > > here.

> Source/WebCore/css/CSSParserValues.cpp:109
> +void CSSParserSelector::adoptSelectorVector(Vector<OwnPtr<CSSParserSelector> >& selectorVector, Vector<Vector<OwnPtr<CSSParserSelector> >* >& stackedSelectors)

typedef-ing Vector<OwnPtr<CSSParserSelector> > CSSParserSelectorList might make some of this code more readable.

> Source/WebCore/css/CSSSelector.h:34
> +    class CSSSelectorListWrapper :public RefCounted<CSSSelectorListWrapper> {

space before public - also rename to RefcountedCSSSelectorList or similar.

> Source/WebCore/css/CSSSelector.h:50
> +            CSSSelectorListWrapper(CSSSelectorList* impl)

one-liner.

> Source/WebCore/css/CSSSelector.h:253
> +        CSSSelectorList* selectorList() const { return m_hasRareData && m_data.m_rareData->m_selectorList.get() ? m_data.m_rareData->m_selectorList->get() : 0; }

Remove the first .get to make this code clearer. Second get could probably be renamed too.

[Shane Stephens]

Created attachment 133171 [details]
Patch

[Shane Stephens]

Comment on attachment 129509 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=129509&action=review

>> Source/WebCore/css/CSSParser.h:455
>> +    Vector<OwnPtr<CSSParserSelector> > *m_reusableSelectorVector;
> 
> OwnPtr<Vector<OwnPtr<CSSParserSelector> > > here.

Done.

>> Source/WebCore/css/CSSParserValues.cpp:109
>> +void CSSParserSelector::adoptSelectorVector(Vector<OwnPtr<CSSParserSelector> >& selectorVector, Vector<Vector<OwnPtr<CSSParserSelector> >* >& stackedSelectors)
> 
> typedef-ing Vector<OwnPtr<CSSParserSelector> > CSSParserSelectorList might make some of this code more readable.

Done.

>> Source/WebCore/css/CSSSelector.h:34
>> +    class CSSSelectorListWrapper :public RefCounted<CSSSelectorListWrapper> {
> 
> space before public - also rename to RefcountedCSSSelectorList or similar.

Done.

>> Source/WebCore/css/CSSSelector.h:50
>> +            CSSSelectorListWrapper(CSSSelectorList* impl)
> 
> one-liner.

Done.

>> Source/WebCore/css/CSSSelector.h:253
>> +        CSSSelectorList* selectorList() const { return m_hasRareData && m_data.m_rareData->m_selectorList.get() ? m_data.m_rareData->m_selectorList->get() : 0; }
> 
> Remove the first .get to make this code clearer. Second get could probably be renamed too.

Done. Decided not to rename the second 'get'.

[WebKit Review Bot]

Attachment 133171 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/css3..." exit_code: 1
Source/WebCore/css/CSSSelectorList.cpp:115:  Semicolon defining empty statement for this loop. Use { } instead.  [whitespace/semicolon] [5]
Total errors found: 1 in 18 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Shane Stephens]

I think this patch might be ready for a reviewer to look at it. Style failure is due to using a 

do {
..
} while(..);

construct.

[Build Bot]

Comment on attachment 133171 [details]
Patch

Attachment 133171 [details] did not pass win-ews (win):
Output: http://queues.webkit.org/results/12086752

[Shane Stephens]

Created attachment 133186 [details]
Patch

[WebKit Review Bot]

Attachment 133186 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/css3..." exit_code: 1
Source/WebCore/css/CSSSelectorList.cpp:115:  Semicolon defining empty statement for this loop. Use { } instead.  [whitespace/semicolon] [5]
Total errors found: 1 in 18 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Sam Weinig]

This absolutely needs to be reviewed by Antti before it goes in.

[Shane Stephens]

It'd be great if Antti could do a review!

[Andreas Kling]

Yeah, this is definitely for Antti.

[Alexis Menard (darktears)]

Comment on attachment 133186 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=133186&action=review

> Source/WebCore/css/CSSParser.h:311
> +    OwnPtr<Vector<CSSProperty, 256> > m_parsedProperties;

I'm wondering is this change was really needed right now. It make the diff noisy.

[Shane Stephens]

Comment on attachment 133186 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=133186&action=review

>> Source/WebCore/css/CSSParser.h:311
>> +    OwnPtr<Vector<CSSProperty, 256> > m_parsedProperties;
> 
> I'm wondering is this change was really needed right now. It make the diff noisy.

I needed to convert m_parsedProperties from an in-instance-data value to a pointer to a value, because it needs to be pushed onto and popped off m_stackedParsedProperties (see pushNestedSelector and popNestedSelector above). It seemed sensible to make it an OwnPtr<Vector<CSSProperty, 256> > instead of a naked Vector<CSSProperty, 256>*.

[Divya Manian]

+1 for Antti's review.

[Ojan Vafai]

Comment on attachment 133186 [details]
Patch

Have you considered instead creating a concept of a NestedCSSStyleRule that has a pointer to a parent rule and then having matchSelector know about parent style rules? Then you don't need to do the expanding. I think that approach would make it a ton easier to make the CSSOM bits work eventually.

[Shane Stephens]

Yes, I have considered this approach. I also tried to implement it, and eventually convinced myself and Dimitri Glaskov that it was not appropriate.

The issue is that different combinators or pseudo-classes have different expansion rules - e.g. 
.a .b is re-ordered to select on .b, then check for .a; whereas .a:hover selects on .a, then checks for :hover. These expansions can exist across nesting boundaries (e.g. .a { & .b { ... } } and .a { &:hover { ... { } }). Because nesting can be arbitrarily deep, the expansion can cause quite complex re-ordering. Furthermore, multiple re-orderings can be required for a single set of nested rules (e.g. .a { & .b, &:hover { & .c, &:active {...} } }.

Have a look at the expansion code. Essentially this logic would need to be implemented recursively in the selector matching code if we were to use a NestedCSSStyleRule + pointer approach.

[Shane Stephens]

Created attachment 154542 [details]
Patch

[WebKit Review Bot]

Attachment 154542 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/css3..." exit_code: 1
Source/WebCore/css/CSSSelectorList.cpp:136:  Semicolon defining empty statement for this loop. Use { } instead.  [whitespace/semicolon] [5]
Total errors found: 1 in 28 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Tony Chang]

Comment on attachment 154542 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=154542&action=review

I mostly just skimmed the style in this patch.

I think getting matchSelector to work would be better in the long run, although harder to implement.  For example, you could probably avoid the exponential memory use, although it would still be slow.

> Source/JavaScriptCore/Configurations/FeatureDefines.xcconfig:42
> +ENABLE_CSS_HIERARCHIES = ;

I would make a separate patch that adds the compiler flag and updates all the build systems.

> Source/WebCore/css/CSSSelectorList.cpp:68
> +    if (stackedSelectors.size() > 0)

!stackedSelectors.isEmpty()

> Source/WebCore/css/CSSSelectorList.cpp:81
> +    if (flattenedSize == 1 && (!stackedSelectors.size())) {

Nit: stackedSelectors.isEmpty()

> Source/WebCore/css/CSSSelectorList.cpp:90
> +    // Flatten the selector hierarchy into explodedSelectors. Each explodedSelectors vector is a single
> +    // element from the cartesian product of the sets of selectors at each level in the hierarchy.

It seems like you could easily overflow the heap and explodedResultsSize (e.g., 64 nested hierarchies each with 2 rules).  Should we limit this somehow?

> Source/WebCore/css/CSSSelectorList.cpp:93
> +    for (size_t i = 0; i < stackedSelectors.size(); i++)

Nit: ++i

> Source/WebCore/css/CSSSelectorList.cpp:96
> +    Vector<OwnPtr<CSSSelector> >* explodedSelectors = new Vector<OwnPtr<CSSSelector> >[explodedResultsSize];

Can we use an OwnArrayPtr rather than manually calling delete []?

> Source/WebCore/css/CSSSelectorList.cpp:97
> +    size_t* insertLocations = new size_t[explodedResultsSize];

Can we use a Vector<size_t> here?

> Source/WebCore/css/CSSSelectorList.cpp:98
> +    size_t considered = 1;

Please move this down to where it is assigned (line 109).

> Source/WebCore/css/CSSSelectorList.cpp:102
> +    for (size_t j = 0; j < explodedResultsSize; j++) {
> +        size_t i = j % selectorVector.size();

Please use more descriptive variable names than i and j.  E.g., explodedIndex and selectorIndex,.

> Source/WebCore/css/CSSSelectorList.cpp:121
> +    for (int i = stackedSelectors.size() - 1; i >= 0; i--) {
> +        CSSSelectorVector& currentInStack = *(stackedSelectors[i]);
> +        for (size_t j = 0; j < explodedResultsSize; j++) {
> +            size_t use = (j / considered) % currentInStack.size();

Please use more descriptive variable names than i, j and use.

> Source/WebCore/css/CSSSelectorList.cpp:132
> +                int k = 0;

Please use a better variable name than k.

> Source/WebCore/css/CSSSelectorList.cpp:142
> +    considered *= currentInStack.size();

Nit: Bad indent.

> LayoutTests/css3/simple-hierarchies.html:97
> +if (layoutTestController) {
> +  layoutTestController.dumpAsText();

Nit: layoutTestController -> testRunner.

Why does the expected result have a render tree dump if this is a dumpAsText test?  I would probably make this a JS test and use getComputedStyle to verify that the right style is being applied.

[Shane Stephens]

Comment on attachment 154542 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=154542&action=review

>> Source/JavaScriptCore/Configurations/FeatureDefines.xcconfig:42
>> +ENABLE_CSS_HIERARCHIES = ;
> 
> I would make a separate patch that adds the compiler flag and updates all the build systems.

Done (92433)

>> Source/WebCore/css/CSSSelectorList.cpp:68
>> +    if (stackedSelectors.size() > 0)
> 
> !stackedSelectors.isEmpty()

done

>> Source/WebCore/css/CSSSelectorList.cpp:81
>> +    if (flattenedSize == 1 && (!stackedSelectors.size())) {
> 
> Nit: stackedSelectors.isEmpty()

done

>> Source/WebCore/css/CSSSelectorList.cpp:90
>> +    // element from the cartesian product of the sets of selectors at each level in the hierarchy.
> 
> It seems like you could easily overflow the heap and explodedResultsSize (e.g., 64 nested hierarchies each with 2 rules).  Should we limit this somehow?

I've added a check against static const value so that we don't accumulate more than 2^16 exploded results.

>> Source/WebCore/css/CSSSelectorList.cpp:93
>> +    for (size_t i = 0; i < stackedSelectors.size(); i++)
> 
> Nit: ++i

Done.

>> Source/WebCore/css/CSSSelectorList.cpp:96
>> +    Vector<OwnPtr<CSSSelector> >* explodedSelectors = new Vector<OwnPtr<CSSSelector> >[explodedResultsSize];
> 
> Can we use an OwnArrayPtr rather than manually calling delete []?

Done.

>> Source/WebCore/css/CSSSelectorList.cpp:97
>> +    size_t* insertLocations = new size_t[explodedResultsSize];
> 
> Can we use a Vector<size_t> here?

I ended up using an OwnArrayPtr here too, as we know the size up-front.

>> Source/WebCore/css/CSSSelectorList.cpp:98
>> +    size_t considered = 1;
> 
> Please move this down to where it is assigned (line 109).

Done.

>> Source/WebCore/css/CSSSelectorList.cpp:102
>> +        size_t i = j % selectorVector.size();
> 
> Please use more descriptive variable names than i and j.  E.g., explodedIndex and selectorIndex,.

Done.

>> Source/WebCore/css/CSSSelectorList.cpp:121
>> +            size_t use = (j / considered) % currentInStack.size();
> 
> Please use more descriptive variable names than i, j and use.

Done.

>> Source/WebCore/css/CSSSelectorList.cpp:132
>> +                int k = 0;
> 
> Please use a better variable name than k.

Done.

>> Source/WebCore/css/CSSSelectorList.cpp:136
>> +                while (relation == CSSSelector::SubSelector);
> 
> Semicolon defining empty statement for this loop. Use { } instead.  [whitespace/semicolon] [5]

What should I do about this? It's neater to use a do .. while loop, but this style error is triggered.

>> Source/WebCore/css/CSSSelectorList.cpp:142
>> +    considered *= currentInStack.size();
> 
> Nit: Bad indent.

Done.

>> LayoutTests/css3/simple-hierarchies.html:97
>> +  layoutTestController.dumpAsText();
> 
> Nit: layoutTestController -> testRunner.
> 
> Why does the expected result have a render tree dump if this is a dumpAsText test?  I would probably make this a JS test and use getComputedStyle to verify that the right style is being applied.

Removed. It looks like this wasn't running because there wasn't a </script>. Yuck.

WRT JS test vs how this is now - isn't this a bit cleaner as we don't rely on any JS to do the comparisons? color and bgcolor are both included in the render tree dump.

[Shane Stephens]

Created attachment 154799 [details]
Patch

[WebKit Review Bot]

Attachment 154799 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/css3..." exit_code: 1
Source/WebCore/css/CSSSelectorList.cpp:141:  Semicolon defining empty statement for this loop. Use { } instead.  [whitespace/semicolon] [5]
Total errors found: 1 in 20 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Tony Chang]

Comment on attachment 154799 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=154799&action=review

Some of the code changes (like typedefs for CSSSelectorVector and CSSSelectorVectorList and converting m_parsedProperties to an OwnPtr) make this diff harder to review.  You may want to move them into a separate change or revert them.

> Source/WebCore/css/CSSParser.h:245
> +    PassOwnPtr<CSSSelectorVector > sinkFloatingSelectorVector(CSSSelectorVector*);

Nit: Extra space before >

> Source/WebCore/css/CSSSelectorList.cpp:99
> +        if (explodedResultsSize > maxExplodedResults)
> +            return;

Can you add a test case that hits this?

>> Source/WebCore/css/CSSSelectorList.cpp:141
>> +                while (relation == CSSSelector::SubSelector);
> 
> Semicolon defining empty statement for this loop. Use { } instead.  [whitespace/semicolon] [5]

"If any of these errors are false positives, please file a bug against check-webkit-style."  I would say that this is a false positive.

> LayoutTests/css3/simple-hierarchies-expected.txt:2
> +layer at (0,0) size 800x600
> +  RenderView at (0,0) size 800x600

> WRT JS test vs how this is now - isn't this a bit cleaner as we don't rely on any JS to do the comparisons? color and bgcolor are both included in the render tree dump.

You'll have to check in a separate rendertree output for every port and platform because text sizes aren't exactly the same.  You also will need pixel dumps for each platform.  It's harder to quickly inspect that the test is correct rather than a series of PASS/FAIL being displayed.

I think it would also be fine to have a ref-test to verify that the correct colors are being painted, but it seems like if getComputedStyle and what is painted are different, that's a bug in some other part of the code, not something you introduced.

[Shane Stephens]

Created attachment 156881 [details]
Patch

[WebKit Review Bot]

Attachment 156881 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/css3..." exit_code: 1
Source/WebCore/css/CSSSelectorList.cpp:140:  Semicolon defining empty statement for this loop. Use { } instead.  [whitespace/semicolon] [5]
Total errors found: 1 in 20 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Shane Stephens]

Comment on attachment 154799 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=154799&action=review

>> Source/WebCore/css/CSSParser.h:245
>> +    PassOwnPtr<CSSSelectorVector > sinkFloatingSelectorVector(CSSSelectorVector*);
> 
> Nit: Extra space before >

Done

>> Source/WebCore/css/CSSSelectorList.cpp:99
>> +            return;
> 
> Can you add a test case that hits this?

Done

>>> Source/WebCore/css/CSSSelectorList.cpp:141
>>> +                while (relation == CSSSelector::SubSelector);
>> 
>> Semicolon defining empty statement for this loop. Use { } instead.  [whitespace/semicolon] [5]
> 
> "If any of these errors are false positives, please file a bug against check-webkit-style."  I would say that this is a false positive.

Done

[Shane Stephens]

I've also fixed the layout test to use dumpAsText and check the computedStyle.

[Gyuyoung Kim]

Comment on attachment 156881 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=156881&action=review

> Source/WebCore/css/CSSPageRule.h:-48
> -

IMHO, this line removal is unrelated to this patch.

> Source/WebCore/css/CSSParser.h:315
> +        StackedParsedProperties *stackHead = m_stackedParsedProperties.last();

I think this is wrong '*' place.

> Source/WebCore/css/CSSParser.h:358
> +    Vector<StackedParsedProperties *> m_stackedParsedProperties;

According to my quick search, WebKit coding style didn't use a space between Vector<...>

> Source/WebCore/css/CSSSelector.h:49
> +            CSSSelectorList *m_impl;

This look wrong '*' place.

> Source/WebCore/css/CSSSelector.h:61
> +        CSSSelector(CSSSelector* copy);

It looks *explicit* is missing.

> Source/WebCore/css/CSSSelectorList.cpp:27
> +

IMHO, this is unneeded line.

> LayoutTests/platform/qt/TestExpectations:87
> +BUGWK79939 SKIP : css3/simple-hierarchies.html = PASS

Could you add this skip to LayoutTests/platform/efl/TestExpectations as well ?

[Shane Stephens]

Comment on attachment 156881 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=156881&action=review

>> Source/WebCore/css/CSSPageRule.h:-48
>> -
> 
> IMHO, this line removal is unrelated to this patch.

removed.

>> Source/WebCore/css/CSSParser.h:315
>> +        StackedParsedProperties *stackHead = m_stackedParsedProperties.last();
> 
> I think this is wrong '*' place.

Fixed.

>> Source/WebCore/css/CSSParser.h:358
>> +    Vector<StackedParsedProperties *> m_stackedParsedProperties;
> 
> According to my quick search, WebKit coding style didn't use a space between Vector<...>

Do you mean that this should be Vector<StackedParsedProperties*>? I've fixed it, if so.

>> Source/WebCore/css/CSSSelector.h:49
>> +            CSSSelectorList *m_impl;
> 
> This look wrong '*' place.

Fixed.

>> Source/WebCore/css/CSSSelector.h:61
>> +        CSSSelector(CSSSelector* copy);
> 
> It looks *explicit* is missing.

Added.

>> Source/WebCore/css/CSSSelectorList.cpp:27
>> +
> 
> IMHO, this is unneeded line.

Fixed.

>> LayoutTests/platform/qt/TestExpectations:87
>> +BUGWK79939 SKIP : css3/simple-hierarchies.html = PASS
> 
> Could you add this skip to LayoutTests/platform/efl/TestExpectations as well ?

Added.

[Shane Stephens]

Created attachment 157048 [details]
Patch

[WebKit Review Bot]

Attachment 157048 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/css3..." exit_code: 1
Source/WebCore/css/CSSSelectorList.cpp:139:  Semicolon defining empty statement for this loop. Use { } instead.  [whitespace/semicolon] [5]
Total errors found: 1 in 21 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Tony Chang]

Comment on attachment 157048 [details]
Patch

For those watching this bug:
Shane has been emailing with Antti and Antti's main concern is that the changes to the CSS grammar, parser and data structures are significant and will make understanding this code harder for everyone.

However, he doesn't feel strongly enough that this should block the feature completely, just that we consider it.

The plan going forward is to break this into multiple patches to make it easier to review and to try to make the changes less invasive.

[WebKit Review Bot]

Comment on attachment 157048 [details]
Patch

Attachment 157048 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/13470410

[Antti Koivisto]

(In reply to comment #34)
> (From update of attachment 157048 [details])
> For those watching this bug:
> Shane has been emailing with Antti and Antti's main concern is that the changes to the CSS grammar, parser and data structures are significant and will make understanding this code harder for everyone.

My main concern was really that this feature is in very early stages in the CSSWG doesn't seem that great in general. It would be far less disruptive to experiment in a branch somewhere rather than in mainline WebKit.

[Antti Koivisto]

*and doesn't seem

[Antti Koivisto]

CSS WG conducted a prioritization poll. Hierarchies was placed 47th out of 58 features with 4 very strong/strong interest votes vs. 16 low/no interest votes.

http://disruptive-innovations.com/zoo/customers/CSSWG/Priorities.html

Based on this I don't think this feature should go into WebKit at this time.

[Antti Koivisto]

s/16/12/

[Shane Stephens]

That list is intended as an indication of where to allocate effort when time grows short at CSSWG face-to-face meetings, and should not be used as a prioritization of implementation effort for feature work. A salient example: "sticky positioning" (http://lists.w3.org/Archives/Public/www-style/2012Jun/0627.html, https://bugs.webkit.org/show_bug.cgi?id=90046) is not even included on a specification in that list yet, but is undergoing active development, behind a flag.

Some of the discussion at the WG has been around rolling hierarchies in as a part of Selectors level 4 or 5, which would remove the need for a separate specification. Perhaps this is part of the reason that the _specification_ has been given low priority? Certainly discussions of the _feature_ have engendered a general level of acceptance at the WG.

[Tony Chang]

(In reply to comment #38)
> CSS WG conducted a prioritization poll. Hierarchies was placed 47th out of 58 features with 4 very strong/strong interest votes vs. 16 low/no interest votes.
> 
> http://disruptive-innovations.com/zoo/customers/CSSWG/Priorities.html
> 
> Based on this I don't think this feature should go into WebKit at this time.

I talked to Shane off list and we've agreed to halt development of this feature at this time.  I'd like to see buy-in from at least one other browser vendor.

I'll post some patches to revert the incomplete work.

Thanks!