Bug 95831

Summary: [Shadow DOM][Refactoring] Element subclasses should have a way to reject author shadows.
Product: WebKit Reporter: Hajime Morrita <morrita>
Component: DOMAssignee: Hajime Morrita <morrita>
Status: RESOLVED FIXED    
Severity: Normal CC: cmarcelo, mifenton, tkent, webcomponents-bugzilla, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 94082    
Attachments:
Description Flags
Patch
none
Patch
none
Patch none

Hajime Morrita
Reported 2012-09-05 02:04:06 PDT
Each element subclasses should have a method to reject author shadows to protect its UA shadow. Currently we have such checkin ShadowRoot but it should be handled by each element.
Attachments
Patch (4.54 KB, patch)
2012-09-05 03:20 PDT, Hajime Morrita 		no flags
DetailsFormatted DiffDiff
Patch (4.35 KB, patch)
2012-09-05 03:23 PDT, Hajime Morrita 		no flags
DetailsFormatted DiffDiff
Patch (4.36 KB, patch)
2012-09-05 18:01 PDT, Hajime Morrita 		no flags
DetailsFormatted DiffDiff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
Hajime Morrita
Comment 1 2012-09-05 03:20:12 PDT
Created attachment 162202 [details] Patch
Hajime Morrita
Comment 2 2012-09-05 03:21:45 PDT
Comment on attachment 162202 [details] Patch Attached a wrong patch...
Hajime Morrita
Comment 3 2012-09-05 03:23:20 PDT
Created attachment 162206 [details] Patch
Build Bot
Comment 4 2012-09-05 05:27:58 PDT
Comment on attachment 162206 [details] Patch Attachment 162206 [details] did not pass win-ews (win): Output: http://queues.webkit.org/results/13765174
Dimitri Glazkov (Google)
Comment 5 2012-09-05 08:45:16 PDT
Comment on attachment 162206 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=162206&action=review The change is good, but needs spelling fixes and a rename to plural. > Source/WebCore/ChangeLog:9 > + This chagne moves that responsibility to each Element subclasses to let coming chagne->change, and "to let coming" -> "to make future" > Source/WebCore/ChangeLog:10 > + author shdaow support improvement local to each replaced elements, rather than shdaow->shadow :) > Source/WebCore/html/HTMLInputElement.h:295 > + virtual bool isAuthorShadowsAllowed() const OVERRIDE { return false; } is->are
Hajime Morrita
Comment 6 2012-09-05 18:01:36 PDT
Created attachment 162385 [details] Patch
Hajime Morrita
Comment 7 2012-09-05 18:03:16 PDT
Thanks for take a look, Dimitri! I kicked my ashamed misspelling out. Could you PTAL?
Hajime Morrita
Comment 8 2012-09-06 18:40:21 PDT
Dimitri: ping? I want this to mask some unpolished elements.
Hajime Morrita
Comment 9 2012-09-06 18:50:02 PDT
Comment on attachment 162385 [details] Patch Thanks ;-)
WebKit Review Bot
Comment 10 2012-09-06 19:25:14 PDT
Comment on attachment 162385 [details] Patch Clearing flags on attachment: 162385 Committed r127811: <http://trac.webkit.org/changeset/127811>
WebKit Review Bot
Comment 11 2012-09-06 19:25:18 PDT
All reviewed patches have been landed. Closing bug.
Elliott Sprehn
Comment 12 2012-09-06 22:03:48 PDT
Comment on attachment 162385 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=162385&action=review > Source/WebCore/svg/SVGElement.h:141 > + virtual bool areAuthorShadowsAllowed() const OVERRIDE { return false; } Note that all SVG elements inherit from SVGElement. If this was to return true you'd allow attaching shadows to things like <path> and <rect> which doesn't make any sense. This should only ever return true from SVGSVGElement.h
Hajime Morrita
Comment 13 2012-09-06 22:45:04 PDT
(In reply to comment #12) > > Note that all SVG elements inherit from SVGElement. If this was to return true you'd allow attaching shadows to things like <path> and <rect> which doesn't make any sense. > > This should only ever return true from SVGSVGElement.h Could you elaborate? Currently SVG uses ShadowRoot in some tricky way and we don't make it work with author shadows yet. This is why we prohibit author shadows here. It has been like that from the beginning and this patch just clarifies what we've been doing and has no behavioral change. Does this make sense?
Note You need to log in before you can comment on or make changes to this bug.