Bug 100831

Summary: [Shadow] Element should have getter and setter of attribute 'pseudo'
Product: WebKit Reporter: Shinya Kawanaka <shinyak>
Component: DOMAssignee: Shinya Kawanaka <shinyak>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, cmarcelo, dglazkov, esprehn, haraken, morrita, ojan, webcomponents-bugzilla, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 100812    
Attachments:
Description Flags
WIP
none
Patch
none
Patch none

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
DetailsFormatted DiffDiff
Patch (5.65 KB, patch)
2012-10-31 21:51 PDT, Shinya Kawanaka 		no flags
DetailsFormatted DiffDiff
Patch (5.69 KB, patch)
2012-11-01 18:29 PDT, Shinya Kawanaka 		no flags
DetailsFormatted DiffDiff
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.