[Shadow] Element should have getter and setter of attribute 'pseudo'

[Shinya Kawanaka]

According to Shadow DOM spec
https://dvcs.w3.org/hg/webcomponents/raw-file/tip/spec/shadow/index.html#extensions-to-element
Element should have getter/setter of attribute 'pseudo'.

[Shinya Kawanaka]

Created attachment 171616 [details]
WIP

[Shinya Kawanaka]

+haraken

I would like to return 'null' when pseudo attribute is not set.
How do I write idl?

This WIP patch returns empty string instead of null.

[Kentaro Hara]

(In reply to comment #2)
> +haraken
> 
> I would like to return 'null' when pseudo attribute is not set.
> How do I write idl?
> 
> This WIP patch returns empty string instead of null.

[TreatReturnedNullStringAs=Null] would be helpful:
https://trac.webkit.org/wiki/WebKitIDL#TreatReturnedNullStringAs

[Shinya Kawanaka]

Created attachment 171767 [details]
Patch

[Shinya Kawanaka]

This patch just exposes 'pseudo' attribute.
I would like to merge 'pseudo' attribute and pseudoShadowId in Bug 100451.

[Shinya Kawanaka]

Oops, not Bug 100451 but Bug 100812

[Hajime Morrita]

Comment on attachment 171767 [details]
Patch

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

This might be a question  to the spec rather than to the implementation, but shouldn't we aware of the validity of the value?
Don't we need to reject invalid pseudo values?

> Source/WebCore/dom/Element.idl:122
> +    [TreatReturnedNullStringAs=Null] attribute DOMString pseudo;

Please guard this using [V8EnabledAtRuntime] annotation. Also, I guess you can use [Reflect].

[Dimitri Glazkov (Google)]

(In reply to comment #7)
> (From update of attachment 171767 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=171767&action=review
> 
> This might be a question  to the spec rather than to the implementation, but shouldn't we aware of the validity of the value?
> Don't we need to reject invalid pseudo values?

That's already in the spec. We don't reject invalid values, we just ignore them when matching.

> 
> > Source/WebCore/dom/Element.idl:122
> > +    [TreatReturnedNullStringAs=Null] attribute DOMString pseudo;
> 
> Please guard this using [V8EnabledAtRuntime] annotation. Also, I guess you can use [Reflect].

[Dimitri Glazkov (Google)]

Comment on attachment 171767 [details]
Patch

Consider morrita's comments.

[Shinya Kawanaka]

(In reply to comment #7)
> (From update of attachment 171767 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=171767&action=review
> 
> This might be a question  to the spec rather than to the implementation, but shouldn't we aware of the validity of the value?
> Don't we need to reject invalid pseudo values?
> 
> > Source/WebCore/dom/Element.idl:122
> > +    [TreatReturnedNullStringAs=Null] attribute DOMString pseudo;
> 
> Please guard this using [V8EnabledAtRuntime] annotation. Also, I guess you can use [Reflect].

When I used [Reflect], 'pseudo' returned empty string instead of null...

[Shinya Kawanaka]

Created attachment 171965 [details]
Patch

[Shinya Kawanaka]

Added V8EnabledAtRuntime guard.
I didn't use Reflect because pseudo returns empty string instead of null.

[Hajime Morrita]

(In reply to comment #12)
> Added V8EnabledAtRuntime guard.
> I didn't use Reflect because pseudo returns empty string instead of null.

This smells like the spec is inconsistent with other part of HTML. We might want to reconsider the behavior.

[Hajime Morrita]

Comment on attachment 171965 [details]
Patch

But let's land this for now.

[Shinya Kawanaka]

(In reply to comment #13)
> (In reply to comment #12)
> > Added V8EnabledAtRuntime guard.
> > I didn't use Reflect because pseudo returns empty string instead of null.
> 
> This smells like the spec is inconsistent with other part of HTML. We might want to reconsider the behavior.

I think so, too. I didn't re-read the HTML spec yet though.

[Shinya Kawanaka]

For example, title attribute returns empty string when it's not specified.

http://jsfiddle.net/B3SZT/

[Shinya Kawanaka]

According to this, non-readonly attribute uses DOMString instead of DOMString?.
http://dev.w3.org/html5/spec/elements.html#htmlelement

[Shinya Kawanaka]

Filed https://www.w3.org/Bugs/Public/show_bug.cgi?id=19825 in ShadowDOM spec.

[WebKit Review Bot]

Comment on attachment 171965 [details]
Patch

Clearing flags on attachment: 171965

Committed r133268: <http://trac.webkit.org/changeset/133268>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.

[Elliott Sprehn]

Note that all other attribute getters return empty string (id, className, title, etc.) so we definitely need to fix the spec and our behavior before shipping Shadow DOM publicly.

[Dimitri Glazkov (Google)]

(In reply to comment #21)
> Note that all other attribute getters return empty string (id, className, title, etc.) so we definitely need to fix the spec and our behavior before shipping Shadow DOM publicly.

Okay: https://www.w3.org/Bugs/Public/show_bug.cgi?id=19825

[Dimitri Glazkov (Google)]

(In reply to comment #22)
> (In reply to comment #21)
> > Note that all other attribute getters return empty string (id, className, title, etc.) so we definitely need to fix the spec and our behavior before shipping Shadow DOM publicly.
> 
> Okay: https://www.w3.org/Bugs/Public/show_bug.cgi?id=19825

http://dvcs.w3.org/hg/webcomponents/rev/229cda7a6d0c