Changeset 116508 in webkit


Ignore:
Timestamp:
May 9, 2012 2:26:57 AM (12 years ago)
Author:
shinyak@chromium.org
Message:

Position should be able to have ShadowRoot as a container.
​https://bugs.webkit.org/show_bug.cgi?id=82021

Reviewed by Ryosuke Niwa.

Source/WebCore:

Since Position could not take a shadow root as a container node, pointing the direct children
of a shadow root was difficult.

This patch makes it enabled, and fixes a lot of crashes caused by that limitation.
Also, we confirm that ShadowRoot is not exposed to JavaScript layer.

Currently this change is only enabled if shadow dom flag is enabled, since we cannot
prove this change does not destroy the existing behavior. However, this change is really required
to fix other editing bugs in Shadow DOM. A bunch of patches and tests will be added to
fix other editing bugs and they will check this patch does not break editing.
We will also add a fuzzer to check the stability of editing in Shadow DOM later, and it will
also help to confirm the patch will not break the editing.

Tests: editing/shadow/doubleclick-on-meter-in-shadow-crash.html

editing/shadow/rightclick-on-meter-in-shadow-crash.html
editing/shadow/shadow-selection-not-exported.html

  • dom/Position.cpp:

(WebCore::Position::Position):
(WebCore::Position::containerNode):
(WebCore::Position::parentAnchoredEquivalent):
(WebCore::Position::previous):
(WebCore::Position::next):
(WebCore::Position::atStartOfTree):
(WebCore::Position::atEndOfTree):
(WebCore::Position::findParent):

  • dom/Position.h:

(WebCore):
(WebCore::positionInParentBeforeNode):
(WebCore::positionInParentAfterNode):

LayoutTests:

Skipped non-chromium port since SHADOW_DOM flag is not enabled in those platforms.
We will add bunch of tests for editing in Shadow DOM into editing/shadow later,
so I added the whole directory name to the skipped list.

  • editing/shadow/doubleclick-on-meter-in-shadow-crash-expected.txt: Added.
  • editing/shadow/doubleclick-on-meter-in-shadow-crash.html: Added.
  • editing/shadow/rightclick-on-meter-in-shadow-crash-expected.txt: Added.
  • editing/shadow/rightclick-on-meter-in-shadow-crash.html: Added.
  • editing/shadow/shadow-selection-not-exported-expected.txt: Added.
  • editing/shadow/shadow-selection-not-exported.html: Added.
  • platform/efl/Skipped:
  • platform/gtk/test_expectations.txt:
  • platform/mac/Skipped:
  • platform/qt/Skipped:
  • platform/win/Skipped:
  • platform/wincairo/Skipped:
  • platform/wk2/Skipped:
Location:
trunk
Files:
7 added
11 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r116506 r116508  
     12012-05-09  Shinya Kawanaka  <shinyak@chromium.org>
     2
     3        Position should be able to have ShadowRoot as a container.
     4        https://bugs.webkit.org/show_bug.cgi?id=82021
     5
     6        Reviewed by Ryosuke Niwa.
     7
     8        Skipped non-chromium port since SHADOW_DOM flag is not enabled in those platforms.
     9        We will add bunch of tests for editing in Shadow DOM into editing/shadow later,
     10        so I added the whole directory name to the skipped list.
     11
     12        * editing/shadow/doubleclick-on-meter-in-shadow-crash-expected.txt: Added.
     13        * editing/shadow/doubleclick-on-meter-in-shadow-crash.html: Added.
     14        * editing/shadow/rightclick-on-meter-in-shadow-crash-expected.txt: Added.
     15        * editing/shadow/rightclick-on-meter-in-shadow-crash.html: Added.
     16        * editing/shadow/shadow-selection-not-exported-expected.txt: Added.
     17        * editing/shadow/shadow-selection-not-exported.html: Added.
     18        * platform/efl/Skipped:
     19        * platform/gtk/test_expectations.txt:
     20        * platform/mac/Skipped:
     21        * platform/qt/Skipped:
     22        * platform/win/Skipped:
     23        * platform/wincairo/Skipped:
     24        * platform/wk2/Skipped:
     25
    1262012-05-09  Mikhail Pozdnyakov  <mikhail.pozdnyakov@intel.com>
    227

  • trunk/LayoutTests/platform/efl/Skipped

    r116506 r116508  
    228228# ENABLE(SHADOW_DOM) is disabled
    229229fast/dom/shadow
     230editing/shadow
    230231
    231232# ENABLE(DATALIST) is disabled

  • trunk/LayoutTests/platform/gtk/test_expectations.txt

    r116404 r116508  
    237237// ENABLE(SHADOW_DOM) is disabled.
    238238BUGWKGTK SKIP : fast/dom/shadow = FAIL
     239BUGWKGTK SKIP : editing/shadow = FAIL
    239240
    240241// CSS Regions support not yet enabled.

  • trunk/LayoutTests/platform/mac/Skipped

    r116428 r116508  
    448448# ENABLE(SHADOW_DOM) is disabled.
    449449fast/dom/shadow
     450editing/shadow
    450451
    451452# JSC does not support setIsolatedWorldSecurityOrigin (http://webkit.org/b/61540)

  • trunk/LayoutTests/platform/qt/Skipped

    r116428 r116508  
    170170
    171171# ENABLE(SHADOW_DOM) is disabled.
     172editing/shadow
    172173fast/dom/shadow/access-key.html
    173174fast/dom/shadow/adopt-node-with-shadow-root.html

  • trunk/LayoutTests/platform/win/Skipped

    r116230 r116508  
    14781478
    14791479# ENABLE(SHADOW_DOM) is disabled.
     1480editing/shadow
    14801481fast/dom/shadow/content-element-api.html
    14811482fast/dom/shadow/content-element-outside-shadow.html

  • trunk/LayoutTests/platform/wincairo/Skipped

    r116074 r116508  
    19841984
    19851985# ENABLE(SHADOW_DOM) is disabled.
     1986editing/shadow
    19861987fast/dom/shadow/content-element-api.html
    19871988fast/dom/shadow/content-element-outside-shadow.html

  • trunk/LayoutTests/platform/wk2/Skipped

    r116009 r116508  
    10911091
    10921092# ENABLE(SHADOW_DOM) is disabled.
     1093editing/shadow
    10931094fast/dom/shadow/content-element-api.html
    10941095fast/dom/shadow/content-element-outside-shadow.html

  • trunk/Source/WebCore/ChangeLog

    r116507 r116508  
     12012-05-09  Shinya Kawanaka  <shinyak@chromium.org>
     2
     3        Position should be able to have ShadowRoot as a container.
     4        https://bugs.webkit.org/show_bug.cgi?id=82021
     5
     6        Reviewed by Ryosuke Niwa.
     7
     8        Since Position could not take a shadow root as a container node, pointing the direct children
     9        of a shadow root was difficult.
     10
     11        This patch makes it enabled, and fixes a lot of crashes caused by that limitation.
     12        Also, we confirm that ShadowRoot is not exposed to JavaScript layer.
     13
     14        Currently this change is only enabled if shadow dom flag is enabled, since we cannot
     15        prove this change does not destroy the existing behavior. However, this change is really required
     16        to fix other editing bugs in Shadow DOM. A bunch of patches and tests will be added to
     17        fix other editing bugs and they will check this patch does not break editing.
     18        We will also add a fuzzer to check the stability of editing in Shadow DOM later, and it will
     19        also help to confirm the patch will not break the editing.
     20
     21        Tests: editing/shadow/doubleclick-on-meter-in-shadow-crash.html
     22               editing/shadow/rightclick-on-meter-in-shadow-crash.html
     23               editing/shadow/shadow-selection-not-exported.html
     24
     25        * dom/Position.cpp:
     26        (WebCore::Position::Position):
     27        (WebCore::Position::containerNode):
     28        (WebCore::Position::parentAnchoredEquivalent):
     29        (WebCore::Position::previous):
     30        (WebCore::Position::next):
     31        (WebCore::Position::atStartOfTree):
     32        (WebCore::Position::atEndOfTree):
     33        (WebCore::Position::findParent):
     34        * dom/Position.h:
     35        (WebCore):
     36        (WebCore::positionInParentBeforeNode):
     37        (WebCore::positionInParentAfterNode):
     38
    1392012-05-09  Zoltan Horvath  <zoltan@webkit.org>
    240

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

    r115460 r116508  
    3434#include "RenderBlock.h"
    3535#include "RenderText.h"
     36#include "RuntimeEnabledFeatures.h"
    3637#include "Text.h"
    3738#include "TextIterator.h"
    … …  
    8182    , m_isLegacyEditingPosition(true)
    8283{
     84#if ENABLE(SHADOW_DOM)
     85    ASSERT(RuntimeEnabledFeatures::shadowDOMEnabled() || !m_anchorNode || !m_anchorNode->isShadowRoot());
     86#else
    8387    ASSERT(!m_anchorNode || !m_anchorNode->isShadowRoot());
     88#endif
    8489}
    8590
    … …  
    9095    , m_isLegacyEditingPosition(false)
    9196{
     97#if ENABLE(SHADOW_DOM)
     98    ASSERT(RuntimeEnabledFeatures::shadowDOMEnabled() || !m_anchorNode || !m_anchorNode->isShadowRoot());
     99#else
    92100    ASSERT(!m_anchorNode || !m_anchorNode->isShadowRoot());
     101#endif
     102
    93103    ASSERT(anchorType != PositionIsOffsetInAnchor);
    94104    ASSERT(!((anchorType == PositionIsBeforeChildren || anchorType == PositionIsAfterChildren)
    … …  
    102112    , m_isLegacyEditingPosition(false)
    103113{
     114#if ENABLE(SHADOW_DOM)
     115     ASSERT(RuntimeEnabledFeatures::shadowDOMEnabled() || !m_anchorNode || !editingIgnoresContent(m_anchorNode.get()) || !m_anchorNode->isShadowRoot());
     116#else
    104117    ASSERT(!m_anchorNode || !editingIgnoresContent(m_anchorNode.get()) || !m_anchorNode->isShadowRoot());
     118#endif
     119
    105120    ASSERT(anchorType == PositionIsOffsetInAnchor);
    106121}
    … …  
    144159    case PositionIsBeforeAnchor:
    145160    case PositionIsAfterAnchor:
    146         return m_anchorNode->nonShadowBoundaryParentNode();
     161        return findParent(m_anchorNode.get());
    147162    }
    148163    ASSERT_NOT_REACHED();
    … …  
    204219    // FIXME: This should only be necessary for legacy positions, but is also needed for positions before and after Tables
    205220    if (m_offset <= 0 && (m_anchorType != PositionIsAfterAnchor && m_anchorType != PositionIsAfterChildren)) {
    206         if (m_anchorNode->nonShadowBoundaryParentNode() && (editingIgnoresContent(m_anchorNode.get()) || isTableElement(m_anchorNode.get())))
     221        if (findParent(m_anchorNode.get()) && (editingIgnoresContent(m_anchorNode.get()) || isTableElement(m_anchorNode.get())))
    207222            return positionInParentBeforeNode(m_anchorNode.get());
    208223        return Position(m_anchorNode.get(), 0, PositionIsOffsetInAnchor);
    … …  
    317332    }
    318333
    319     ContainerNode* parent = n->nonShadowBoundaryParentNode();
     334    ContainerNode* parent = findParent(n);
    320335    if (!parent)
    321336        return *this;
    … …  
    349364    }
    350365
    351     ContainerNode* parent = n->nonShadowBoundaryParentNode();
     366    ContainerNode* parent = findParent(n);
    352367    if (!parent)
    353368        return *this;
    … …  
    442457    if (isNull())
    443458        return true;
    444     return !deprecatedNode()->nonShadowBoundaryParentNode() && m_offset <= 0;
     459    return !findParent(deprecatedNode()) && m_offset <= 0;
    445460}
    446461
    … …  
    449464    if (isNull())
    450465        return true;
    451     return !deprecatedNode()->nonShadowBoundaryParentNode() && m_offset >= lastOffsetForEditing(deprecatedNode());
     466    return !findParent(deprecatedNode()) && m_offset >= lastOffsetForEditing(deprecatedNode());
    452467}
    453468
    … …  
    833848{
    834849    return node && node->renderer() && node->renderer()->style()->userSelect() == SELECT_NONE;
     850}
     851
     852ContainerNode* Position::findParent(const Node* node)
     853{
     854    // FIXME: See http://web.ug/82697
     855
     856#if ENABLE(SHADOW_DOM)
     857    if (RuntimeEnabledFeatures::shadowDOMEnabled())
     858        return node->parentNode();
     859#endif
     860
     861    return node->nonShadowBoundaryParentNode();
    835862}
    836863

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

    r115460 r116508  
    190190    static bool hasRenderedNonAnonymousDescendantsWithHeight(RenderObject*);
    191191    static bool nodeIsUserSelectNone(Node*);
     192
     193    static ContainerNode* findParent(const Node*);
    192194   
    193195    void debugPosition(const char* msg = "") const;
    … …  
    204206    int renderedOffset() const;
    205207
     208   
    206209    Position previousCharacterPosition(EAffinity) const;
    207210    Position nextCharacterPosition(EAffinity) const;
    … …  
    245248    // that the caller is trying to make a position relative to a disconnected node (which is likely an error)
    246249    // Specifically, editing/deleting/delete-ligature-001.html crashes with ASSERT(node->parentNode())
    247     return Position(node->nonShadowBoundaryParentNode(), node->nodeIndex(), Position::PositionIsOffsetInAnchor);
     250    return Position(Position::findParent(node), node->nodeIndex(), Position::PositionIsOffsetInAnchor);
    248251}
    249252
    250253inline Position positionInParentAfterNode(const Node* node)
    251254{
    252     ASSERT(node->nonShadowBoundaryParentNode());
    253     return Position(node->nonShadowBoundaryParentNode(), node->nodeIndex() + 1, Position::PositionIsOffsetInAnchor);
     255    ASSERT(Position::findParent(node));
     256    return Position(Position::findParent(node), node->nodeIndex() + 1, Position::PositionIsOffsetInAnchor);
    254257}
    255258
Note: See TracChangeset for help on using the changeset viewer.