- Revision
- 120887
- Author
- [email protected]
- Date
- 2012-06-20 17:10:42 -0700 (Wed, 20 Jun 2012)
Log Message
[Shadow][Editing] Selection will break editing boundaries in Shadow DOM.
https://bugs.webkit.org/show_bug.cgi?id=89075
Reviewed by Antti Koivisto.
Source/WebCore:
At the shadow boundary, some styles (e.g. user-modify, text-decoration) cannot be inherited
from the shadow host. However, when style property cache is used, such styles are wrongly
inherited from the cache.
So this patch makes not to use cache for inehrited styles at the shadow boundary.
Tests: editing/shadow/breaking-editing-boundary-with-table.html
editing/shadow/contenteditable-propagation-at-shadow-boundary.html
* css/StyleResolver.cpp:
(WebCore::StyleResolver::collectMatchingRulesForList):
LayoutTests:
Test cases to confirm that (1) selection does not break editing boundary,
and (2) content-editable is not propagated from shadow host to shadow DOM.
Actually a patch for Bug 88514 contains a test for (2), but it is not enough.
* editing/shadow/breaking-editing-boundary-with-table.html: Added.
* editing/shadow/contenteditable-propagation-at-shadow-boundary.html: Added.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (120886 => 120887)
--- trunk/LayoutTests/ChangeLog 2012-06-20 23:55:18 UTC (rev 120886)
+++ trunk/LayoutTests/ChangeLog 2012-06-21 00:10:42 UTC (rev 120887)
@@ -1,3 +1,19 @@
+2012-06-20 Shinya Kawanaka <[email protected]>
+
+ [Shadow][Editing] Selection will break editing boundaries in Shadow DOM.
+ https://bugs.webkit.org/show_bug.cgi?id=89075
+
+ Reviewed by Antti Koivisto.
+
+ Test cases to confirm that (1) selection does not break editing boundary,
+ and (2) content-editable is not propagated from shadow host to shadow DOM.
+
+ Actually a patch for Bug 88514 contains a test for (2), but it is not enough.
+
+
+ * editing/shadow/breaking-editing-boundary-with-table.html: Added.
+ * editing/shadow/contenteditable-propagation-at-shadow-boundary.html: Added.
+
2012-06-20 Adam Barth <[email protected]>
internals.settings.setDeviceScaleFactor doesn't work for WebKit2 and must be removed
Added: trunk/LayoutTests/editing/shadow/breaking-editing-boundary-with-table-expected.txt (0 => 120887)
--- trunk/LayoutTests/editing/shadow/breaking-editing-boundary-with-table-expected.txt (rev 0)
+++ trunk/LayoutTests/editing/shadow/breaking-editing-boundary-with-table-expected.txt 2012-06-21 00:10:42 UTC (rev 120887)
@@ -0,0 +1,7 @@
+On the second time selecting <span>, it should not become non-contenteditable. If it becomes non-contenteditable, the selection from <span> to <table> will break editing boundaries and contain "a". This test checks the <span> does not changed to non-contenteditable element.
+
+PASS contains(selectedString, "a") is false
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
Added: trunk/LayoutTests/editing/shadow/breaking-editing-boundary-with-table.html (0 => 120887)
--- trunk/LayoutTests/editing/shadow/breaking-editing-boundary-with-table.html (rev 0)
+++ trunk/LayoutTests/editing/shadow/breaking-editing-boundary-with-table.html 2012-06-21 00:10:42 UTC (rev 120887)
@@ -0,0 +1,56 @@
+<!DOCTYPE html>
+
+<html>
+<body>
+<script src=""
+<script src=""
+<script src=""
+
+<p>On the second time selecting <span>, it should not become non-contenteditable.
+If it becomes non-contenteditable, the selection from <span> to <table>
+will break editing boundaries and contain "a".
+This test checks the <span> does not changed to non-contenteditable element.</p>
+
+<div id="host" contenteditable></div>
+<pre id="console"></pre>
+
+<script>
+var shadowRoot = new WebKitShadowRoot(host);
+shadowRoot.innerHTML = '<table border="1">' +
+ '<tr><td id="dst">aaaaa</td><td id="another">aaaaa</td></tr>' +
+ '<tr><td>aaaaa</td><td>aaaaa</td></tr>' +
+ '</table><span id="src" contenteditable>bbbbb</span>';
+
+var src = ""
+var dst = shadowRoot.getElementById('dst');
+var another = shadowRoot.getElementById('another');
+
+// 1. Select from src to dst.
+mouseMoveToElem(src);
+eventSender.mouseDown();
+mouseMoveToElem(dst);
+eventSender.mouseUp();
+
+// 2. Click some element.
+mouseMoveToElem(another);
+eventSender.mouseDown();
+eventSender.mouseUp();
+
+// 3. Select from src to dst again.
+mouseMoveToElem(src);
+eventSender.mouseDown();
+mouseMoveToElem(dst);
+eventSender.mouseUp();
+
+function contains(str, key) {
+ return str.indexOf(key) >= 0;
+}
+
+var selectedString = shadowRoot.getSelection().toString();
+shouldBeFalse('contains(selectedString, "a")');
+
+var successfullyParsed = true;
+</script>
+<script src=""
+</body>
+</html>
Added: trunk/LayoutTests/editing/shadow/contenteditable-propagation-at-shadow-boundary-expected.txt (0 => 120887)
--- trunk/LayoutTests/editing/shadow/contenteditable-propagation-at-shadow-boundary-expected.txt (rev 0)
+++ trunk/LayoutTests/editing/shadow/contenteditable-propagation-at-shadow-boundary-expected.txt 2012-06-21 00:10:42 UTC (rev 120887)
@@ -0,0 +1,10 @@
+This test checks that content-editable is not propagated from shadow host to a shadow subtree.
+
+This p is required to produce the issue.
+
+This div and parent div are required to produce the issue.
+PASS computedStyle(nodeInShadowRoot1, userModifyPropertyName) is "read-only"
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
Added: trunk/LayoutTests/editing/shadow/contenteditable-propagation-at-shadow-boundary.html (0 => 120887)
--- trunk/LayoutTests/editing/shadow/contenteditable-propagation-at-shadow-boundary.html (rev 0)
+++ trunk/LayoutTests/editing/shadow/contenteditable-propagation-at-shadow-boundary.html 2012-06-21 00:10:42 UTC (rev 120887)
@@ -0,0 +1,50 @@
+<!DOCTYPE html>
+<html>
+<head>
+<script src=""
+<script src=""
+</head>
+<body>
+
+<p>This test checks that content-editable is not propagated from shadow host to a shadow subtree.</p>
+
+<p>This p is required to produce the issue.</p>
+<div contenteditable>
+ <div>This div and parent div are required to produce the issue.</div>
+ <div id="host1" contenteditable>shadow host 1</div>
+</div>
+<pre id="console"></pre>
+<script>
+if (window.layoutTestController)
+ layoutTestController.dumpAsText();
+
+function computedStyle(element, style) {
+ var cs = window.getComputedStyle(element);
+ if (!cs)
+ return '(NA)';
+ if (cs.getPropertyCSSValue(style))
+ return cs.getPropertyCSSValue(style).cssText;
+ return undefined;
+}
+
+function prepareNodeInShadowRoot(host) {
+ var shadowRoot = new WebKitShadowRoot(host);
+ var nodeInShadow = document.createElement('div');
+ nodeInShadow.setAttribute('id', 'node-in-shadow-root');
+ nodeInShadow.appendChild(document.createTextNode('In Shadow'));
+ shadowRoot.appendChild(nodeInShadow);
+ return nodeInShadow;
+}
+
+document.body.offsetLeft;
+
+var nodeInShadowRoot1 = prepareNodeInShadowRoot(document.getElementById('host1'));
+var userModifyPropertyName = '-webkit-user-modify';
+shouldBeEqualToString('computedStyle(nodeInShadowRoot1, userModifyPropertyName)', 'read-only');
+
+var successfullyParsed = true;
+</script>
+
+<script src=""
+</body>
+</html>
Modified: trunk/Source/WebCore/ChangeLog (120886 => 120887)
--- trunk/Source/WebCore/ChangeLog 2012-06-20 23:55:18 UTC (rev 120886)
+++ trunk/Source/WebCore/ChangeLog 2012-06-21 00:10:42 UTC (rev 120887)
@@ -1,3 +1,22 @@
+2012-06-20 Shinya Kawanaka <[email protected]>
+
+ [Shadow][Editing] Selection will break editing boundaries in Shadow DOM.
+ https://bugs.webkit.org/show_bug.cgi?id=89075
+
+ Reviewed by Antti Koivisto.
+
+ At the shadow boundary, some styles (e.g. user-modify, text-decoration) cannot be inherited
+ from the shadow host. However, when style property cache is used, such styles are wrongly
+ inherited from the cache.
+
+ So this patch makes not to use cache for inehrited styles at the shadow boundary.
+
+ Tests: editing/shadow/breaking-editing-boundary-with-table.html
+ editing/shadow/contenteditable-propagation-at-shadow-boundary.html
+
+ * css/StyleResolver.cpp:
+ (WebCore::StyleResolver::collectMatchingRulesForList):
+
2012-06-20 Garret Kelly <[email protected]>
Moving cookieEnabled/setCookieEnabled from Page to Settings.
Modified: trunk/Source/WebCore/css/StyleResolver.cpp (120886 => 120887)
--- trunk/Source/WebCore/css/StyleResolver.cpp 2012-06-20 23:55:18 UTC (rev 120886)
+++ trunk/Source/WebCore/css/StyleResolver.cpp 2012-06-21 00:10:42 UTC (rev 120887)
@@ -2949,11 +2949,12 @@
// style declarations. We then only need to apply the inherited properties, if any, as their values can depend on the
// element context. This is fast and saves memory by reusing the style data structures.
m_style->copyNonInheritedFrom(cacheItem->renderStyle.get());
- if (m_parentStyle->inheritedDataShared(cacheItem->parentRenderStyle.get())) {
+ if (m_parentStyle->inheritedDataShared(cacheItem->parentRenderStyle.get()) && !isAtShadowBoundary(element)) {
EInsideLink linkStatus = m_style->insideLink();
// If the cache item parent style has identical inherited properties to the current parent style then the
// resulting style will be identical too. We copy the inherited properties over from the cache and are done.
- m_style->inheritFrom(cacheItem->renderStyle.get(), isAtShadowBoundary(element) ? RenderStyle::AtShadowBoundary : RenderStyle::NotAtShadowBoundary);
+ m_style->inheritFrom(cacheItem->renderStyle.get());
+
// Unfortunately the link status is treated like an inherited property. We need to explicitly restore it.
m_style->setInsideLink(linkStatus);
return;
