2011-04-04 Chang Shu <cshu@webkit.org>
authorcommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 4 Apr 2011 22:49:22 +0000 (22:49 +0000)
committercommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 4 Apr 2011 22:49:22 +0000 (22:49 +0000)
        Reviewed by Ryosuke Niwa.

        setContentEditable with true/false/inherit string is not working properly
        https://bugs.webkit.org/show_bug.cgi?id=52058

        Updated expected results after this patch fixes the set contenteditable issue.

        * fast/dom/HTMLElement/set-false-expected.txt:
        * fast/dom/HTMLElement/set-false.html:
        * fast/dom/HTMLElement/set-inherit-parent-false-expected.txt:
        * fast/dom/HTMLElement/set-inherit-parent-true-expected.txt:
        * fast/dom/HTMLElement/set-true-expected.txt:
        * fast/dom/HTMLElement/set-value-caseinsensitive-expected.txt:
2011-04-04  Chang Shu  <cshu@webkit.org>

        Reviewed by Ryosuke Niwa.

        setContentEditable with true/false/inherit string is not working properly
        https://bugs.webkit.org/show_bug.cgi?id=52058

        Move isContentEditable from HTMLElement to Node. Thus, Node provides two functions for
        checking editability: rendererIsEditable and isContentEdiable. The former is a fast path,
        which does NOT trigger layout and only checks the render style of usermodify. The latter
        updates the layout first to make sure the render style syncs with DOM contenteditable
        attribute. Certain call sites that need to call isContentEditable rather than rendererIsEditable
        are also updated in the patch. But a complete fix will follow up in bug 57244.

        This patch fixes all the failed layout tests related to set contenteditable.

        * accessibility/AccessibilityRenderObject.cpp:
        (WebCore::AccessibilityRenderObject::isReadOnly):
        * dom/Node.cpp:
        (WebCore::Node::isContentEditable):
        (WebCore::Node::shouldUseInputMethod):
        * dom/Node.h:
        * html/HTMLElement.cpp:
        * html/HTMLElement.h:
2011-04-04  Chang Shu  <cshu@webkit.org>

        Reviewed by Ryosuke Niwa.

        setContentEditable with true/false/inherit string is not working properly
        https://bugs.webkit.org/show_bug.cgi?id=52058

        Move isContentEditable from HTMLElement to Node. WebKit should only access isContentEditable
        as rendererIsEditable is for WebCore internal use.

        * src/WebNode.cpp:
        (WebKit::WebNode::isContentEditable):
        * src/WebViewImpl.cpp:
        (WebKit::WebViewImpl::setFocus):
        (WebKit::WebViewImpl::setComposition):
        (WebKit::WebViewImpl::confirmComposition):
2011-04-04  Chang Shu  <cshu@webkit.org>

        Reviewed by Ryosuke Niwa.

        setContentEditable with true/false/inherit string is not working properly
        https://bugs.webkit.org/show_bug.cgi?id=52058

        Move isContentEditable from HTMLElement to Node. WebKit should only access isContentEditable
        as rendererIsEditable is for WebCore internal use.

        * WebCoreSupport/EditorClientHaiku.cpp:
        (WebCore::EditorClientHaiku::handleKeyboardEvent):
2011-04-04  Chang Shu  <cshu@webkit.org>

        Reviewed by Ryosuke Niwa.

        setContentEditable with true/false/inherit string is not working properly
        https://bugs.webkit.org/show_bug.cgi?id=52058

        Move isContentEditable from HTMLElement to Node. WebKit should only access isContentEditable
        as rendererIsEditable is for WebCore internal use.

        * WebCoreSupport/EditorClientQt.cpp:
        (WebCore::EditorClientQt::handleKeyboardEvent):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@82886 268f45cc-cd09-0410-ab3c-d52691b4dbfc

20 files changed:
LayoutTests/ChangeLog
LayoutTests/fast/dom/HTMLElement/set-false-expected.txt
LayoutTests/fast/dom/HTMLElement/set-false.html
LayoutTests/fast/dom/HTMLElement/set-inherit-parent-false-expected.txt
LayoutTests/fast/dom/HTMLElement/set-inherit-parent-true-expected.txt
LayoutTests/fast/dom/HTMLElement/set-true-expected.txt
LayoutTests/fast/dom/HTMLElement/set-value-caseinsensitive-expected.txt
Source/WebCore/ChangeLog
Source/WebCore/accessibility/AccessibilityRenderObject.cpp
Source/WebCore/dom/Node.cpp
Source/WebCore/dom/Node.h
Source/WebCore/html/HTMLElement.cpp
Source/WebCore/html/HTMLElement.h
Source/WebKit/chromium/ChangeLog
Source/WebKit/chromium/src/WebNode.cpp
Source/WebKit/chromium/src/WebViewImpl.cpp
Source/WebKit/haiku/ChangeLog
Source/WebKit/haiku/WebCoreSupport/EditorClientHaiku.cpp
Source/WebKit/qt/ChangeLog
Source/WebKit/qt/WebCoreSupport/EditorClientQt.cpp

index 5e5531a..657ae71 100644 (file)
@@ -1,3 +1,19 @@
+2011-04-04  Chang Shu  <cshu@webkit.org>
+
+        Reviewed by Ryosuke Niwa.
+
+        setContentEditable with true/false/inherit string is not working properly
+        https://bugs.webkit.org/show_bug.cgi?id=52058
+
+        Updated expected results after this patch fixes the set contenteditable issue.
+
+        * fast/dom/HTMLElement/set-false-expected.txt:
+        * fast/dom/HTMLElement/set-false.html:
+        * fast/dom/HTMLElement/set-inherit-parent-false-expected.txt:
+        * fast/dom/HTMLElement/set-inherit-parent-true-expected.txt:
+        * fast/dom/HTMLElement/set-true-expected.txt:
+        * fast/dom/HTMLElement/set-value-caseinsensitive-expected.txt:
+
 2011-04-04  MORITA Hajime  <morrita@google.com>
 
         Reviewed by Dimitri Glazkov.
index fb42bf5..a3d3aed 100644 (file)
@@ -9,8 +9,7 @@ On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE
 
 PASS document.getElementById("div1").getAttribute("contentEditable") is "false"
 PASS document.getElementById("div1").contentEditable is "false"
-FAIL document.getElementById("div1").isContentEditable should be false. Was true.
-FIXME: setContentEditable with true/false/inherit string is not working properly. https://bugs.webkit.org/show_bug.cgi?id=52058
+PASS document.getElementById("div1").isContentEditable is false
 PASS window.getComputedStyle(div1, "").getPropertyValue("-webkit-user-modify") is "read-only"
 PASS document.getElementById("p2").contentEditable is "false"
 PASS document.getElementById("p2").isContentEditable is false
index 6351515..8bf5d1d 100644 (file)
@@ -21,7 +21,6 @@ document.getElementById("p2").contentEditable = "false";
 shouldBe('document.getElementById("div1").getAttribute("contentEditable")','"false"');
 shouldBe('document.getElementById("div1").contentEditable', '"false"');
 shouldBe('document.getElementById("div1").isContentEditable', 'false');
-debug("FIXME: setContentEditable with true/false/inherit string is not working properly. https://bugs.webkit.org/show_bug.cgi?id=52058");
 shouldBe('window.getComputedStyle(div1, "").getPropertyValue("-webkit-user-modify")', '"read-only"');
 
 shouldBe('document.getElementById("p2").contentEditable', '"false"');
index 8033e25..badb394 100644 (file)
@@ -8,7 +8,7 @@ On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE
 
 PASS document.getElementById("p").hasAttribute("contentEditable") is false
 PASS document.getElementById("p").contentEditable is "inherit"
-FAIL document.getElementById("p").isContentEditable should be false. Was true.
+PASS document.getElementById("p").isContentEditable is false
 FIXME: setContentEditable with true/false/inherit string is not working properly. https://bugs.webkit.org/show_bug.cgi?id=52058
 PASS window.getComputedStyle(p, "").getPropertyValue("-webkit-user-modify") is "read-only"
 
index 3b6f0bd..0ed0036 100644 (file)
@@ -8,7 +8,7 @@ On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE
 
 PASS document.getElementById("p").hasAttribute("contentEditable") is false
 PASS document.getElementById("p").contentEditable is "inherit"
-FAIL document.getElementById("p").isContentEditable should be true. Was false.
+PASS document.getElementById("p").isContentEditable is true
 FIXME: setContentEditable with true/false/inherit string is not working properly. https://bugs.webkit.org/show_bug.cgi?id=52058
 PASS window.getComputedStyle(p, "").getPropertyValue("-webkit-user-modify") is "read-write"
 
index 52ba0fa..1a7464e 100644 (file)
@@ -9,7 +9,7 @@ On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE
 
 PASS document.getElementById("div1").getAttribute("contentEditable") is "true"
 PASS document.getElementById("div1").contentEditable is "true"
-FAIL document.getElementById("div1").isContentEditable should be true. Was false.
+PASS document.getElementById("div1").isContentEditable is true
 FIXME: setContentEditable with true/false/inherit string is not working properly. https://bugs.webkit.org/show_bug.cgi?id=52058
 PASS window.getComputedStyle(div1, "").getPropertyValue("-webkit-user-modify") is "read-write"
 PASS document.getElementById("p2").getAttribute("contentEditable") is "true"
index 070e82f..d6b2980 100644 (file)
@@ -11,7 +11,7 @@ On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE
 
 PASS document.getElementById("div1").getAttribute("contentEditable") is "true"
 PASS document.getElementById("div1").contentEditable is "true"
-FAIL document.getElementById("div1").isContentEditable should be true. Was false.
+PASS document.getElementById("div1").isContentEditable is true
 FIXME: isContentEditable is not working properly. Related to https://bugs.webkit.org/show_bug.cgi?id=52058
 PASS window.getComputedStyle(div1, "").getPropertyValue("-webkit-user-modify") is "read-write"
 PASS document.getElementById("p1").getAttribute("contentEditable") is "false"
index 388da7c..0d4cd07 100644 (file)
@@ -1,3 +1,28 @@
+2011-04-04  Chang Shu  <cshu@webkit.org>
+
+        Reviewed by Ryosuke Niwa.
+
+        setContentEditable with true/false/inherit string is not working properly
+        https://bugs.webkit.org/show_bug.cgi?id=52058
+
+        Move isContentEditable from HTMLElement to Node. Thus, Node provides two functions for
+        checking editability: rendererIsEditable and isContentEdiable. The former is a fast path,
+        which does NOT trigger layout and only checks the render style of usermodify. The latter
+        updates the layout first to make sure the render style syncs with DOM contenteditable 
+        attribute. Certain call sites that need to call isContentEditable rather than rendererIsEditable
+        are also updated in the patch. But a complete fix will follow up in bug 57244.
+
+        This patch fixes all the failed layout tests related to set contenteditable.
+
+        * accessibility/AccessibilityRenderObject.cpp:
+        (WebCore::AccessibilityRenderObject::isReadOnly):
+        * dom/Node.cpp:
+        (WebCore::Node::isContentEditable):
+        (WebCore::Node::shouldUseInputMethod):
+        * dom/Node.h:
+        * html/HTMLElement.cpp:
+        * html/HTMLElement.h:
+
 2011-04-04  Roland Steiner  <rolandsteiner@chromium.org>
 
         Reviewed by Dimitri Glazkov.
index 9b54a69..d53c9e2 100644 (file)
@@ -661,7 +661,7 @@ bool AccessibilityRenderObject::isReadOnly() const
             return true;
         
         HTMLElement* body = document->body();
-        if (body && body->rendererIsEditable())
+        if (body && body->isContentEditable())
             return false;
 
         return !document->rendererIsEditable();
index b346064..fb474f3 100644 (file)
@@ -709,6 +709,12 @@ void Node::deprecatedParserAddChild(PassRefPtr<Node>)
 {
 }
 
+bool Node::isContentEditable() const
+{
+    document()->updateLayoutIgnorePendingStylesheets();
+    return rendererIsEditable(Editable);
+}
+
 bool Node::rendererIsEditable(EditableLevel editableLevel) const
 {
     if (document()->inDesignMode() || (document()->frame() && document()->frame()->page() && document()->frame()->page()->isEditable()))
@@ -738,7 +744,7 @@ bool Node::rendererIsEditable(EditableLevel editableLevel) const
 
 bool Node::shouldUseInputMethod() const
 {
-    return rendererIsEditable();
+    return isContentEditable();
 }
 
 RenderBox* Node::renderBox() const
index 129a493..b94ff1a 100644 (file)
@@ -319,10 +319,8 @@ public:
     virtual bool isKeyboardFocusable(KeyboardEvent*) const;
     virtual bool isMouseFocusable() const;
 
-#if PLATFORM(MAC)
-    // Objective-C extensions
-    bool isContentEditable() const { return rendererIsEditable(Editable); }
-#endif
+    bool isContentEditable() const;
+
     bool rendererIsEditable() const { return rendererIsEditable(Editable); }
     bool rendererIsRichlyEditable() const { return rendererIsEditable(RichlyEditable); }
     virtual bool shouldUseInputMethod() const;
index e3b5043..aede4bd 100644 (file)
@@ -657,11 +657,6 @@ bool HTMLElement::supportsFocus() const
     return Element::supportsFocus() || (rendererIsEditable() && parentNode() && !parentNode()->rendererIsEditable());
 }
 
-bool HTMLElement::isContentEditable() const
-{
-    return rendererIsEditable();
-}
-
 String HTMLElement::contentEditable() const
 {
     const AtomicString& value = fastGetAttribute(contenteditableAttr);
index 077b1c3..b2926d1 100644 (file)
@@ -57,8 +57,6 @@ public:
 
     virtual bool supportsFocus() const;
 
-    bool isContentEditable() const;
-
     String contentEditable() const;
     void setContentEditable(const String&, ExceptionCode&);
 
index acf0a35..2979dbb 100644 (file)
@@ -1,3 +1,20 @@
+2011-04-04  Chang Shu  <cshu@webkit.org>
+
+        Reviewed by Ryosuke Niwa.
+
+        setContentEditable with true/false/inherit string is not working properly
+        https://bugs.webkit.org/show_bug.cgi?id=52058
+
+        Move isContentEditable from HTMLElement to Node. WebKit should only access isContentEditable
+        as rendererIsEditable is for WebCore internal use.
+
+        * src/WebNode.cpp:
+        (WebKit::WebNode::isContentEditable):
+        * src/WebViewImpl.cpp:
+        (WebKit::WebViewImpl::setFocus):
+        (WebKit::WebViewImpl::setComposition):
+        (WebKit::WebViewImpl::confirmComposition):
+
 2011-04-04  Alexey Proskuryakov  <ap@apple.com>
 
         Reviewed by Dan Bernstein.
index 68b6f13..cfb8528 100644 (file)
@@ -152,7 +152,7 @@ bool WebNode::isFocusable() const
 
 bool WebNode::isContentEditable() const
 {
-    return m_private->rendererIsEditable();
+    return m_private->isContentEditable();
 }
 
 bool WebNode::isElementNode() const
index 0515668..669efaf 100644 (file)
@@ -1268,7 +1268,7 @@ void WebViewImpl::setFocus(bool enable)
                 Element* element = static_cast<Element*>(focusedNode);
                 if (element->isTextFormControl())
                     element->updateFocusAppearance(true);
-                else if (focusedNode->rendererIsEditable()) {
+                else if (focusedNode->isContentEditable()) {
                     // updateFocusAppearance() selects all the text of
                     // contentseditable DIVs. So we set the selection explicitly
                     // instead. Note that this has the side effect of moving the
@@ -1330,7 +1330,7 @@ bool WebViewImpl::setComposition(
     PassRefPtr<Range> range = editor->compositionRange();
     if (range) {
         const Node* node = range->startContainer();
-        if (!node || !node->rendererIsEditable())
+        if (!node || !node->isContentEditable())
             return false;
     }
 
@@ -1379,7 +1379,7 @@ bool WebViewImpl::confirmComposition(const WebString& text)
     PassRefPtr<Range> range = editor->compositionRange();
     if (range) {
         const Node* node = range->startContainer();
-        if (!node || !node->rendererIsEditable())
+        if (!node || !node->isContentEditable())
             return false;
     }
 
index 996a1f4..10e537c 100644 (file)
@@ -1,3 +1,16 @@
+2011-04-04  Chang Shu  <cshu@webkit.org>
+
+        Reviewed by Ryosuke Niwa.
+
+        setContentEditable with true/false/inherit string is not working properly
+        https://bugs.webkit.org/show_bug.cgi?id=52058
+
+        Move isContentEditable from HTMLElement to Node. WebKit should only access isContentEditable
+        as rendererIsEditable is for WebCore internal use.
+
+        * WebCoreSupport/EditorClientHaiku.cpp:
+        (WebCore::EditorClientHaiku::handleKeyboardEvent):
+
 2011-03-31  Evan Martin  <evan@chromium.org>
 
         Reviewed by Eric Seidel.
index 1a84dab..5c1b13f 100644 (file)
@@ -254,7 +254,7 @@ void EditorClientHaiku::handleKeyboardEvent(KeyboardEvent* event)
     if (!start)
         return;
 
-    if (start->rendererIsEditable()) {
+    if (start->isContentEditable()) {
         switch (kevent->windowsVirtualKeyCode()) {
         case VK_BACK:
             frame->editor()->deleteWithDirection(DirectionBackward,
index 65c4ee9..73aa0a3 100644 (file)
@@ -1,3 +1,16 @@
+2011-04-04  Chang Shu  <cshu@webkit.org>
+
+        Reviewed by Ryosuke Niwa.
+
+        setContentEditable with true/false/inherit string is not working properly
+        https://bugs.webkit.org/show_bug.cgi?id=52058
+
+        Move isContentEditable from HTMLElement to Node. WebKit should only access isContentEditable
+        as rendererIsEditable is for WebCore internal use.
+
+        * WebCoreSupport/EditorClientQt.cpp:
+        (WebCore::EditorClientQt::handleKeyboardEvent):
+
 2011-04-01  Carol Szabo  <carol.szabo@nokia.com>
 
         Reviewed by Benjamin Poulain.
index e7bbd2c..0110144 100644 (file)
@@ -420,7 +420,7 @@ void EditorClientQt::handleKeyboardEvent(KeyboardEvent* event)
         return;
 
     // FIXME: refactor all of this to use Actions or something like them
-    if (start->rendererIsEditable()) {
+    if (start->isContentEditable()) {
         bool doSpatialNavigation = false;
         if (isSpatialNavigationEnabled(frame)) {
             if (!kevent->modifiers()) {