HTML5 <details> and <summary> initial implementation

• Component: WebCore Misc.
• Status: RESOLVED FIXED

HTML5 <details> and <summary> elements initial implementation. 

The main objective is to add the files for html elements and renderers, and to get rid of build system issues in future patches.

Created an attachment (id=75248) [details]
patch

Attachment 75248 [details] did not build on gtk:
Build output: http://queues.webkit.org/results/6678001

Specification:

http://www.w3.org/TR/html5/interactive-elements.html#the-details-element
http://www.w3.org/TR/html5/interactive-elements.html#the-summary-element

Created an attachment (id=75427) [details]
patch

(From update of attachment 75427 [details])
View in context: https://bugs.webkit.org/attachment.cgi?id=75427&action=review

I don’t want to see these renderer classes added until we are sure we need them. Are you certain you will need all three?

> WebCore/html/HTMLDetailsElement.cpp:55
> +bool HTMLDetailsElement::open() const
> +{
> +    return !getAttribute(openAttr).isNull();
> +}
> +
> +void HTMLDetailsElement::setOpen(bool value)
> +{
> +    setAttribute(openAttr, value ? "" : 0);
> +}

These functions are not needed.

> WebCore/html/HTMLDetailsElement.h:31
> +    static PassRefPtr<HTMLDetailsElement> create(Document* document);

We should not add this function unless we have a callsite that needs it.

> WebCore/html/HTMLDetailsElement.h:34
> +    bool open() const;
> +    void setOpen(bool);

These functions are not needed.

> WebCore/html/HTMLDetailsElement.idl:23
> +        attribute boolean open;

This should use [Reflect] so we don’t need open and canOpen functions in the C++ class.

> WebCore/html/HTMLTagNames.in:118
> -summary interfaceName=HTMLElement
> +summary

This change is incorrect. See the HTML5 specification. It specifically says the DOM interface uses HTMLElement. The class HTMLSummaryElement is not needed.

(In reply to comment #5)
> (From update of attachment 75427 [details] [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=75427&action=review
> 
> I don’t want to see these renderer classes added until we are sure we need them. Are you certain you will need all three?

The plan is to RenderDetails to use RenderBlock::layoutSpecialExcludedChild() to set the position of its first RenderSummary child. And this first RenderSummary creates a RenderDetailsMarker in a way similar to what is done by RenderListItem.

What do you think?

Created an attachment (id=75459) [details]
patch

Attachment 75459 [details] did not build on win:
Build output: http://queues.webkit.org/results/6727025

Created an attachment (id=75464) [details]
patch

Attachment 75464 [details] did not build on win:
Build output: http://queues.webkit.org/results/6736030

(From update of attachment 75464 [details])
I’m still not sure I understand if this will require all three unique renderer classes. At the moment we are just checking them in as dead code which seems a little weak.

r=me, but please find out why the Windows build is failing and resolve that before checking in

Created an attachment (id=75639) [details]
checking build systems before landing. please do not review.

Committed r73346: <http://trac.webkit.org/changeset/73346>