Changeset 82886 in webkit


Ignore:
Timestamp:
Apr 4, 2011 3:49:22 PM (13 years ago)
Author:
commit-queue@webkit.org
Message:

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 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):
Location:
trunk
Files:
20 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r82880 r82886  
     12011-04-04  Chang Shu  <cshu@webkit.org>
     2
     3        Reviewed by Ryosuke Niwa.
     4
     5        setContentEditable with true/false/inherit string is not working properly
     6        https://bugs.webkit.org/show_bug.cgi?id=52058
     7
     8        Updated expected results after this patch fixes the set contenteditable issue.
     9
     10        * fast/dom/HTMLElement/set-false-expected.txt:
     11        * fast/dom/HTMLElement/set-false.html:
     12        * fast/dom/HTMLElement/set-inherit-parent-false-expected.txt:
     13        * fast/dom/HTMLElement/set-inherit-parent-true-expected.txt:
     14        * fast/dom/HTMLElement/set-true-expected.txt:
     15        * fast/dom/HTMLElement/set-value-caseinsensitive-expected.txt:
     16
    1172011-04-04  MORITA Hajime  <morrita@google.com>
    218

  • trunk/LayoutTests/fast/dom/HTMLElement/set-false-expected.txt

    r76528 r82886  
    1010PASS document.getElementById("div1").getAttribute("contentEditable") is "false"
    1111PASS document.getElementById("div1").contentEditable is "false"
    12 FAIL document.getElementById("div1").isContentEditable should be false. Was true.
    13 FIXME: setContentEditable with true/false/inherit string is not working properly. https://bugs.webkit.org/show_bug.cgi?id=52058
     12PASS document.getElementById("div1").isContentEditable is false
    1413PASS window.getComputedStyle(div1, "").getPropertyValue("-webkit-user-modify") is "read-only"
    1514PASS document.getElementById("p2").contentEditable is "false"

  • trunk/LayoutTests/fast/dom/HTMLElement/set-false.html

    r76528 r82886  
    2222shouldBe('document.getElementById("div1").contentEditable', '"false"');
    2323shouldBe('document.getElementById("div1").isContentEditable', 'false');
    24 debug("FIXME: setContentEditable with true/false/inherit string is not working properly. https://bugs.webkit.org/show_bug.cgi?id=52058");
    2524shouldBe('window.getComputedStyle(div1, "").getPropertyValue("-webkit-user-modify")', '"read-only"');
    2625

  • trunk/LayoutTests/fast/dom/HTMLElement/set-inherit-parent-false-expected.txt

    r76528 r82886  
    99PASS document.getElementById("p").hasAttribute("contentEditable") is false
    1010PASS document.getElementById("p").contentEditable is "inherit"
    11 FAIL document.getElementById("p").isContentEditable should be false. Was true.
     11PASS document.getElementById("p").isContentEditable is false
    1212FIXME: setContentEditable with true/false/inherit string is not working properly. https://bugs.webkit.org/show_bug.cgi?id=52058
    1313PASS window.getComputedStyle(p, "").getPropertyValue("-webkit-user-modify") is "read-only"

  • trunk/LayoutTests/fast/dom/HTMLElement/set-inherit-parent-true-expected.txt

    r76528 r82886  
    99PASS document.getElementById("p").hasAttribute("contentEditable") is false
    1010PASS document.getElementById("p").contentEditable is "inherit"
    11 FAIL document.getElementById("p").isContentEditable should be true. Was false.
     11PASS document.getElementById("p").isContentEditable is true
    1212FIXME: setContentEditable with true/false/inherit string is not working properly. https://bugs.webkit.org/show_bug.cgi?id=52058
    1313PASS window.getComputedStyle(p, "").getPropertyValue("-webkit-user-modify") is "read-write"

  • trunk/LayoutTests/fast/dom/HTMLElement/set-true-expected.txt

    r76528 r82886  
    1010PASS document.getElementById("div1").getAttribute("contentEditable") is "true"
    1111PASS document.getElementById("div1").contentEditable is "true"
    12 FAIL document.getElementById("div1").isContentEditable should be true. Was false.
     12PASS document.getElementById("div1").isContentEditable is true
    1313FIXME: setContentEditable with true/false/inherit string is not working properly. https://bugs.webkit.org/show_bug.cgi?id=52058
    1414PASS window.getComputedStyle(div1, "").getPropertyValue("-webkit-user-modify") is "read-write"

  • trunk/LayoutTests/fast/dom/HTMLElement/set-value-caseinsensitive-expected.txt

    r77985 r82886  
    1212PASS document.getElementById("div1").getAttribute("contentEditable") is "true"
    1313PASS document.getElementById("div1").contentEditable is "true"
    14 FAIL document.getElementById("div1").isContentEditable should be true. Was false.
     14PASS document.getElementById("div1").isContentEditable is true
    1515FIXME: isContentEditable is not working properly. Related to https://bugs.webkit.org/show_bug.cgi?id=52058
    1616PASS window.getComputedStyle(div1, "").getPropertyValue("-webkit-user-modify") is "read-write"

  • trunk/Source/WebCore/ChangeLog

    r82882 r82886  
     12011-04-04  Chang Shu  <cshu@webkit.org>
     2
     3        Reviewed by Ryosuke Niwa.
     4
     5        setContentEditable with true/false/inherit string is not working properly
     6        https://bugs.webkit.org/show_bug.cgi?id=52058
     7
     8        Move isContentEditable from HTMLElement to Node. Thus, Node provides two functions for
     9        checking editability: rendererIsEditable and isContentEdiable. The former is a fast path,
     10        which does NOT trigger layout and only checks the render style of usermodify. The latter
     11        updates the layout first to make sure the render style syncs with DOM contenteditable
     12        attribute. Certain call sites that need to call isContentEditable rather than rendererIsEditable
     13        are also updated in the patch. But a complete fix will follow up in bug 57244.
     14
     15        This patch fixes all the failed layout tests related to set contenteditable.
     16
     17        * accessibility/AccessibilityRenderObject.cpp:
     18        (WebCore::AccessibilityRenderObject::isReadOnly):
     19        * dom/Node.cpp:
     20        (WebCore::Node::isContentEditable):
     21        (WebCore::Node::shouldUseInputMethod):
     22        * dom/Node.h:
     23        * html/HTMLElement.cpp:
     24        * html/HTMLElement.h:
     25
    1262011-04-04  Roland Steiner  <rolandsteiner@chromium.org>
    227

  • trunk/Source/WebCore/accessibility/AccessibilityRenderObject.cpp

    r82459 r82886  
    662662       
    663663        HTMLElement* body = document->body();
    664         if (body && body->rendererIsEditable())
     664        if (body && body->isContentEditable())
    665665            return false;
    666666

  • trunk/Source/WebCore/dom/Node.cpp

    r82882 r82886  
    710710}
    711711
     712bool Node::isContentEditable() const
     713{
     714    document()->updateLayoutIgnorePendingStylesheets();
     715    return rendererIsEditable(Editable);
     716}
     717
    712718bool Node::rendererIsEditable(EditableLevel editableLevel) const
    713719{
    … …  
    739745bool Node::shouldUseInputMethod() const
    740746{
    741     return rendererIsEditable();
     747    return isContentEditable();
    742748}
    743749

  • trunk/Source/WebCore/dom/Node.h

    r82127 r82886  
    320320    virtual bool isMouseFocusable() const;
    321321
    322 #if PLATFORM(MAC)
    323     // Objective-C extensions
    324     bool isContentEditable() const { return rendererIsEditable(Editable); }
    325 #endif
     322    bool isContentEditable() const;
     323
    326324    bool rendererIsEditable() const { return rendererIsEditable(Editable); }
    327325    bool rendererIsRichlyEditable() const { return rendererIsEditable(RichlyEditable); }

  • trunk/Source/WebCore/html/HTMLElement.cpp

    r81965 r82886  
    658658}
    659659
    660 bool HTMLElement::isContentEditable() const
    661 {
    662     return rendererIsEditable();
    663 }
    664 
    665660String HTMLElement::contentEditable() const
    666661{

  • trunk/Source/WebCore/html/HTMLElement.h

    r81965 r82886  
    5757
    5858    virtual bool supportsFocus() const;
    59 
    60     bool isContentEditable() const;
    6159
    6260    String contentEditable() const;

  • trunk/Source/WebKit/chromium/ChangeLog

    r82870 r82886  
     12011-04-04  Chang Shu  <cshu@webkit.org>
     2
     3        Reviewed by Ryosuke Niwa.
     4
     5        setContentEditable with true/false/inherit string is not working properly
     6        https://bugs.webkit.org/show_bug.cgi?id=52058
     7
     8        Move isContentEditable from HTMLElement to Node. WebKit should only access isContentEditable
     9        as rendererIsEditable is for WebCore internal use.
     10
     11        * src/WebNode.cpp:
     12        (WebKit::WebNode::isContentEditable):
     13        * src/WebViewImpl.cpp:
     14        (WebKit::WebViewImpl::setFocus):
     15        (WebKit::WebViewImpl::setComposition):
     16        (WebKit::WebViewImpl::confirmComposition):
     17
    1182011-04-04  Alexey Proskuryakov  <ap@apple.com>
    219

  • trunk/Source/WebKit/chromium/src/WebNode.cpp

    r81965 r82886  
    153153bool WebNode::isContentEditable() const
    154154{
    155     return m_private->rendererIsEditable();
     155    return m_private->isContentEditable();
    156156}
    157157

  • trunk/Source/WebKit/chromium/src/WebViewImpl.cpp

    r82870 r82886  
    12691269                if (element->isTextFormControl())
    12701270                    element->updateFocusAppearance(true);
    1271                 else if (focusedNode->rendererIsEditable()) {
     1271                else if (focusedNode->isContentEditable()) {
    12721272                    // updateFocusAppearance() selects all the text of
    12731273                    // contentseditable DIVs. So we set the selection explicitly
    … …  
    13311331    if (range) {
    13321332        const Node* node = range->startContainer();
    1333         if (!node || !node->rendererIsEditable())
     1333        if (!node || !node->isContentEditable())
    13341334            return false;
    13351335    }
    … …  
    13801380    if (range) {
    13811381        const Node* node = range->startContainer();
    1382         if (!node || !node->rendererIsEditable())
     1382        if (!node || !node->isContentEditable())
    13831383            return false;
    13841384    }

  • trunk/Source/WebKit/haiku/ChangeLog

    r82580 r82886  
     12011-04-04  Chang Shu  <cshu@webkit.org>
     2
     3        Reviewed by Ryosuke Niwa.
     4
     5        setContentEditable with true/false/inherit string is not working properly
     6        https://bugs.webkit.org/show_bug.cgi?id=52058
     7
     8        Move isContentEditable from HTMLElement to Node. WebKit should only access isContentEditable
     9        as rendererIsEditable is for WebCore internal use.
     10
     11        * WebCoreSupport/EditorClientHaiku.cpp:
     12        (WebCore::EditorClientHaiku::handleKeyboardEvent):
     13
    1142011-03-31  Evan Martin  <evan@chromium.org>
    215

  • trunk/Source/WebKit/haiku/WebCoreSupport/EditorClientHaiku.cpp

    r81965 r82886  
    255255        return;
    256256
    257     if (start->rendererIsEditable()) {
     257    if (start->isContentEditable()) {
    258258        switch (kevent->windowsVirtualKeyCode()) {
    259259        case VK_BACK:

  • trunk/Source/WebKit/qt/ChangeLog

    r82676 r82886  
     12011-04-04  Chang Shu  <cshu@webkit.org>
     2
     3        Reviewed by Ryosuke Niwa.
     4
     5        setContentEditable with true/false/inherit string is not working properly
     6        https://bugs.webkit.org/show_bug.cgi?id=52058
     7
     8        Move isContentEditable from HTMLElement to Node. WebKit should only access isContentEditable
     9        as rendererIsEditable is for WebCore internal use.
     10
     11        * WebCoreSupport/EditorClientQt.cpp:
     12        (WebCore::EditorClientQt::handleKeyboardEvent):
     13
    1142011-04-01  Carol Szabo  <carol.szabo@nokia.com>
    215

  • trunk/Source/WebKit/qt/WebCoreSupport/EditorClientQt.cpp

    r82243 r82886  
    421421
    422422    // FIXME: refactor all of this to use Actions or something like them
    423     if (start->rendererIsEditable()) {
     423    if (start->isContentEditable()) {
    424424        bool doSpatialNavigation = false;
    425425        if (isSpatialNavigationEnabled(frame)) {
Note: See TracChangeset for help on using the changeset viewer.