Bug 103711

Summary: [Shadow DOM]: reset-style-inheritance doesn't work for insertion point
Product: WebKit Reporter: Sergey G. Grekhov <sgrekhov>
Component: CSSAssignee: Takashi Sakamoto <tasak>
Status: RESOLVED FIXED    
Severity: Normal CC: morrita, ojan.autocc, ojan, webcomponents-bugzilla, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 63606, 103230, 103709    
Attachments:
Description Flags
Patch
none
Patch
none
Patch none

Sergey G. Grekhov
Reported 2012-11-29 22:11:20 PST
According the Shadow DOM spec (https://dvcs.w3.org/hg/webcomponents/raw-file/tip/spec/shadow/index.html#styles) reset-style-inheritance flag should reset all inheritable CSS properties to their initial values at the lower boundary of the tree. This doesn't work in Chrome 23.0.1271.91 m. Example: <html> <head> <script type="text/javascript"> function test() { var SR = window.ShadowRoot || window.WebKitShadowRoot; var d = document; d.body.innerHTML = '<ul id="shHost">' + '<li id="li1" class="shadow">1</li>' + '<li id="li2" class="shadow2">2</li>' + '<li id="li3" class="shadow">3</li>' + '<li id="li4">4</li>' + '<li id="li5" class="shadow">5</li>' + '<li id="li6" class="shadow2">6</li>' + '</ul>'; var host = d.querySelector('#shHost'); d.body.setAttribute('style', 'color:red'); //Shadow root to play with var s = new SR(host); var div = d.createElement('div'); div.innerHTML ='<ul><content select=".shadow" reset-style-inheritance=true></content></ul>'; s.appendChild(div); } </script> </head> <body onload="test()"> </body> </html> Run the example above and observe the browser window. Red color style inherited from document body is not reset in spite of reset-style-inheritance=true attribute. If to modify example above to set resetStyleInheritance property this won't work anyway. In other words: var div = d.createElement('div'); div.innerHTML ='<ul><content select=".shadow" id="shInsPoint"></content></ul>'; s.appendChild(div); s.querySelector('#shInsPoint').resetStyleInheritance = true; The modified script doesn't work anyway
Attachments
Patch (9.14 KB, patch)
2012-12-05 17:26 PST, Takashi Sakamoto 		no flags
DetailsFormatted DiffDiff
Patch (10.25 KB, patch)
2012-12-06 19:05 PST, Takashi Sakamoto 		no flags
DetailsFormatted DiffDiff
Patch (10.27 KB, patch)
2012-12-09 21:42 PST, Takashi Sakamoto 		no flags
DetailsFormatted DiffDiff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
Sergey G. Grekhov
Comment 1 2012-11-29 22:14:03 PST
See also https://bugs.webkit.org/show_bug.cgi?id=103709
Takashi Sakamoto
Comment 2 2012-12-05 17:26:45 PST
Created attachment 177881 [details] Patch
Takashi Sakamoto
Comment 3 2012-12-05 17:28:24 PST
This patch will also fix bug 103709, [Shadow DOM]: reset-style-inheritance flag doesn't work for shadow insertion point.
Hajime Morrita
Comment 4 2012-12-05 20:48:20 PST
Comment on attachment 177881 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=177881&action=review > Source/WebCore/html/shadow/InsertionPoint.cpp:168 > +void InsertionPoint::parseAttribute(const QualifiedName& name, const AtomicString& value) This isn't enough. We need to reflect reflect change on resetStyleInheritance property to the attribute value. In general, we no longer need a boolean flag on the class. That can be represented as an attribute value. See how HTMLButtonElement#disabled is implemented.
Takashi Sakamoto
Comment 5 2012-12-06 19:05:08 PST
Created attachment 178135 [details] Patch
Takashi Sakamoto
Comment 6 2012-12-06 19:34:22 PST
Comment on attachment 177881 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=177881&action=review Thank you for reviewing. >> Source/WebCore/html/shadow/InsertionPoint.cpp:168 >> +void InsertionPoint::parseAttribute(const QualifiedName& name, const AtomicString& value) > > This isn't enough. We need to reflect reflect change on resetStyleInheritance property to the attribute value. > In general, we no longer need a boolean flag on the class. That can be represented as an attribute value. > See how HTMLButtonElement#disabled is implemented. I see. I removed m_shouldResetStyleInheritance and modified to look up at reset-style-inheritance attribute value.
WebKit Review Bot
Comment 7 2012-12-07 00:05:20 PST
Comment on attachment 178135 [details] Patch Rejecting attachment 178135 [details] from commit-queue. Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '-..." exit_code: 2 Last 500 characters of output: dow/InsertionPoint.cpp.rej patching file Source/WebCore/html/shadow/InsertionPoint.h patching file LayoutTests/ChangeLog Hunk #1 succeeded at 1 with fuzz 3. patching file LayoutTests/fast/dom/shadow/insertion-point-resetStyleInheritance-expected.txt patching file LayoutTests/fast/dom/shadow/insertion-point-resetStyleInheritance.html Failed to run "[u'/mnt/git/webkit-commit-queue/Tools/Scripts/svn-apply', u'--force', u'--reviewer', u'Hajime Mor..." exit_code: 1 cwd: /mnt/git/webkit-commit-queue Full output: http://queues.webkit.org/results/15210033
Takashi Sakamoto
Comment 8 2012-12-09 21:42:35 PST
Created attachment 178469 [details] Patch
WebKit Review Bot
Comment 9 2012-12-09 23:44:41 PST
Comment on attachment 178469 [details] Patch Clearing flags on attachment: 178469 Committed r137112: <http://trac.webkit.org/changeset/137112>
WebKit Review Bot
Comment 10 2012-12-09 23:44:45 PST
All reviewed patches have been landed. Closing bug.
Hajime Morrita
Comment 11 2013-01-17 22:30:39 PST
(In reply to comment #10) > All reviewed patches have been landed. Closing bug. It looks test on shadow dom test suite continues failing even after this change. Filed 107229.
Note You need to log in before you can comment on or make changes to this bug.