[webkit-reviews] review denied: [Bug 93922] [Shadow DOM] Insertion points need resetStyleInheritance : [Attachment 167694] Patch

[bugzilla-daemon at webkit.org]

Dimitri Glazkov (Google) <dglazkov at chromium.org> has denied Takashi Sakamoto
<tasak at google.com>'s request for review:
Bug 93922: [Shadow DOM] Insertion points need resetStyleInheritance
https://bugs.webkit.org/show_bug.cgi?id=93922

Attachment 167694: Patch
https://bugs.webkit.org/attachment.cgi?id=167694&action=review

------- Additional Comments from Dimitri Glazkov (Google)
<dglazkov at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=167694&action=review


This is getting close. We need to get CSS WG to go ahead and spec display:
contents, so that we can redefine <content>/<shadow> in terms of that
primitive, instead of the hacking this into StyleResolver.

> Source/WebCore/html/shadow/HTMLShadowElement.idl:37
> +	   attribute boolean resetStyleInheritance;

What about adding the attribute to HTML?

>> Source/WebCore/html/shadow/InsertionPoint.h:81
>> +	bool m_resetStyleInheritance : 1;
> 
> It’s unfortunate that the shadow specification gives a property a verb phrase
name. The function resetStyleInheritance sounds like something that would
perform a reset operation, not a getter that returns a boolean.
> 
> We don’t have to repeat that mistake in our data member. I would suggest the
name “m_shouldResetStyleInheritance” or at least “m_resetsStyleInheritance”,
and also suggest discussing changing the name in the spec.

For what it's worth, since it reflects the "reset-style-inheritance" attribute
on HTMLContentElement/HTMLShadowElement, I think the name falls well into the
current naming convention for attributes, such as "autoplay", "autofocus",
"defer", etc.
http://www.whatwg.org/specs/web-apps/current-work/multipage/section-index.html#
attributes-1