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

RESOLVED FIXED 100831

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

https://bugs.webkit.org/show_bug.cgi?id=100831

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

Shinya Kawanaka

Reported 2012-10-31 02:10:45 PDT

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'.

Attachments
WIP (4.75 KB, patch) 2012-10-31 04:21 PDT, Shinya Kawanaka	no flags	Details Formatted Diff Diff
Patch (5.65 KB, patch) 2012-10-31 21:51 PDT, Shinya Kawanaka	no flags	Details Formatted Diff Diff
Patch (5.69 KB, patch) 2012-11-01 18:29 PDT, Shinya Kawanaka	no flags	Details Formatted Diff Diff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.

Shinya Kawanaka

Comment 1 2012-10-31 04:21:00 PDT

Created attachment 171616 [details] WIP

Shinya Kawanaka

Comment 2 2012-10-31 04:22:01 PDT

+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

Comment 3 2012-10-31 05:23:35 PDT

(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

Comment 4 2012-10-31 21:51:59 PDT

Created attachment 171767 [details] Patch

Shinya Kawanaka

Comment 5 2012-10-31 21:52:46 PDT

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

Shinya Kawanaka

Comment 6 2012-10-31 21:53:22 PDT

Oops, not Bug 100451 but Bug 100812

Hajime Morrita

Comment 7 2012-11-01 03:10:41 PDT

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)

Comment 8 2012-11-01 09:43:40 PDT

(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 9 2012-11-01 09:45:00 PDT

Comment on attachment 171767 [details] Patch Consider morrita's comments.

Shinya Kawanaka

Comment 10 2012-11-01 18:08:12 PDT

(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

Comment 11 2012-11-01 18:29:32 PDT

Created attachment 171965 [details] Patch

Shinya Kawanaka

Comment 12 2012-11-01 18:30:39 PDT

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

Hajime Morrita

Comment 13 2012-11-01 19:01:27 PDT

(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 14 2012-11-01 19:02:05 PDT

Comment on attachment 171965 [details] Patch But let's land this for now.

Shinya Kawanaka

Comment 15 2012-11-01 19:04:29 PDT

(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

Comment 16 2012-11-01 19:21:29 PDT

For example, title attribute returns empty string when it's not specified. http://jsfiddle.net/B3SZT/

Shinya Kawanaka

Comment 17 2012-11-01 19:26:03 PDT

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

Shinya Kawanaka

Comment 18 2012-11-01 19:30:25 PDT

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

WebKit Review Bot

Comment 19 2012-11-01 23:35:49 PDT

Comment on attachment 171965 [details] Patch Clearing flags on attachment: 171965 Committed r133268: <http://trac.webkit.org/changeset/133268>

WebKit Review Bot

Comment 20 2012-11-01 23:35:54 PDT

All reviewed patches have been landed. Closing bug.

Elliott Sprehn

Comment 21 2012-11-19 00:08:31 PST

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)

Comment 22 2012-11-19 09:26:44 PST

(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)

Comment 23 2012-11-19 15:23:06 PST

(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

Note You need to log in before you can comment on or make changes to this bug.

Status RESOLVED

Resolution FIXED

Priority P2

Severity Normal

Classification Unclassified

Version 528+ (Nightly build)

Hardware Unspecified

OS Unspecified

Product WebKit

Component DOM

Assignee

Shinya Kawanaka

Reported

2012-10-31 02:10 PDT

Modified

2012-11-19 15:23 PST History

CC List

9 users Show

URL

Keywords

Depends on

Blocks

100812

Dependencies

tree graph