Bug 1457026: Teach nsDocumentEncoder about display: contents. r=mats
authorEmilio Cobos Álvarez <emilio@crisal.io>
Thu, 26 Apr 2018 02:14:08 +0200
changeset 460789 422aebe40856002e8e53916433703840dc6fafa9
parent 460788 28c2803c243aaccac97653e21013450e74f4ef90
child 460790 46864f33859eecba12b01b4011d392ee20520648
push id165
push userfmarier@mozilla.com
push dateMon, 30 Apr 2018 23:50:51 +0000
reviewersmats
bugs1457026
milestone61.0a1
Bug 1457026: Teach nsDocumentEncoder about display: contents. r=mats MozReview-Commit-ID: 5F3qurRHMNM
dom/base/Element.h
dom/base/nsDocumentEncoder.cpp
editor/libeditor/tests/test_bug525389.html
layout/base/nsCSSFrameConstructor.cpp
layout/generic/nsFrameSelection.cpp
--- a/dom/base/Element.h
+++ b/dom/base/Element.h
@@ -69,24 +69,27 @@ namespace css {
   struct URLValue;
 } // namespace css
 namespace dom {
   struct AnimationFilter;
   struct ScrollIntoViewOptions;
   struct ScrollToOptions;
   class DOMIntersectionObserver;
   class DOMMatrixReadOnly;
+  class Element;
   class ElementOrCSSPseudoElement;
   class UnrestrictedDoubleOrKeyframeAnimationOptions;
   enum class CallerType : uint32_t;
   typedef nsDataHashtable<nsRefPtrHashKey<DOMIntersectionObserver>, int32_t>
     IntersectionObserverList;
 } // namespace dom
 } // namespace mozilla
 
+// Declared here because of include hell.
+extern "C" bool Servo_Element_IsDisplayContents(const mozilla::dom::Element*);
 
 already_AddRefed<nsContentList>
 NS_GetContentList(nsINode* aRootNode,
                   int32_t  aMatchNameSpaceId,
                   const nsAString& aTagname);
 
 #define ELEMENT_FLAG_BIT(n_) NODE_FLAG_BIT(NODE_TYPE_SPECIFIC_BITS_OFFSET + (n_))
 
@@ -1430,16 +1433,21 @@ public:
    * @param aType the kind of flush to do, typically FlushType::Frames or
    *              FlushType::Layout
    * @return the primary frame
    */
   nsIFrame* GetPrimaryFrame(FlushType aType);
   // Work around silly C++ name hiding stuff
   nsIFrame* GetPrimaryFrame() const { return nsIContent::GetPrimaryFrame(); }
 
+  bool IsDisplayContents() const
+  {
+    return HasServoData() && Servo_Element_IsDisplayContents(this);
+  }
+
   const nsAttrValue* GetParsedAttr(nsAtom* aAttr) const
   {
     return mAttrsAndChildren.GetAttr(aAttr);
   }
 
   const nsAttrValue* GetParsedAttr(nsAtom* aAttr, int32_t aNameSpaceID) const
   {
     return mAttrsAndChildren.GetAttr(aAttr, aNameSpaceID);
--- a/dom/base/nsDocumentEncoder.cpp
+++ b/dom/base/nsDocumentEncoder.cpp
@@ -100,26 +100,35 @@ protected:
 
   bool IsVisibleNode(nsINode* aNode)
   {
     NS_PRECONDITION(aNode, "");
 
     if (mFlags & SkipInvisibleContent) {
       // Treat the visibility of the ShadowRoot as if it were
       // the host content.
-      nsCOMPtr<nsIContent> content = do_QueryInterface(aNode);
-      if (ShadowRoot* shadowRoot = ShadowRoot::FromNodeOrNull(content)) {
-        content = shadowRoot->GetHost();
+      //
+      // FIXME(emilio): I suspect instead of this a bunch of the GetParent()
+      // calls here should be doing GetFlattenedTreeParent, then this condition
+      // should be unreachable...
+      if (ShadowRoot* shadowRoot = ShadowRoot::FromNode(aNode)) {
+        aNode = shadowRoot->GetHost();
       }
 
-      if (content) {
-        nsIFrame* frame = content->GetPrimaryFrame();
+      if (aNode->IsContent()) {
+        nsIFrame* frame = aNode->AsContent()->GetPrimaryFrame();
         if (!frame) {
+          if (aNode->IsElement() && aNode->AsElement()->IsDisplayContents()) {
+            return true;
+          }
           if (aNode->IsText()) {
             // We have already checked that our parent is visible.
+            //
+            // FIXME(emilio): Text not assigned to a <slot> in Shadow DOM should
+            // probably return false...
             return true;
           }
           if (aNode->IsHTMLElement(nsGkAtoms::rp)) {
             // Ruby parentheses are part of ruby structure, hence
             // shouldn't be stripped out even if it is not displayed.
             return true;
           }
           return false;
@@ -1243,21 +1252,24 @@ nsHTMLCopyEncoder::SetSelection(nsISelec
   // if selection is uninitialized return
   if (!rangeCount) {
     return NS_ERROR_FAILURE;
   }
 
   // we'll just use the common parent of the first range.  Implicit assumption
   // here that multi-range selections are table cell selections, in which case
   // the common parent is somewhere in the table and we don't really care where.
+  //
+  // FIXME(emilio, bug 1455894): This assumption is already wrong, and will
+  // probably be more wrong in a Shadow DOM world...
+  //
+  // We should be able to write this as "Find the common ancestor of the
+  // selection, then go through the flattened tree and serialize the selected
+  // nodes", effectively serializing the composed tree.
   RefPtr<nsRange> range = selection->GetRangeAt(0);
-  if (!range) {
-    // XXXbz can this happen given rangeCount > 0?
-    return NS_ERROR_NULL_POINTER;
-  }
   nsINode* commonParent = range->GetCommonAncestor();
 
   for (nsCOMPtr<nsIContent> selContent(do_QueryInterface(commonParent));
        selContent;
        selContent = selContent->GetParent())
   {
     // checking for selection inside a plaintext form widget
     if (selContent->IsAnyOfHTMLElements(nsGkAtoms::input, nsGkAtoms::textarea))
--- a/editor/libeditor/tests/test_bug525389.html
+++ b/editor/libeditor/tests/test_bug525389.html
@@ -168,16 +168,22 @@ async function runTest() {
 //  verifyContent('<ol><li id="paste_here">X</li><li>Hello Kitty</li><span>Hello</span></ol>');
 
   await copyToClipBoard("<pre>Kitty</pre><span>Hello</span>", true);
   trans = getTransferableFromClipboard(true);
   pasteInto(trans, '<pre id="paste_here">Hello </pre>',"paste_here");
   verifyContent('<pre id="paste_here">Hello Kitty<span>Hello</span></pre>');
   is(pasteCount, 1, "paste event was not triggered");
 
+  await copyToClipBoard('1<span style="display: contents">2</span>3', true);
+  trans = getTransferableFromClipboard(true);
+  pasteInto(trans, '<div id="paste_here"></div>',"paste_here");
+  verifyContent('<div id="paste_here">1<span style="display: contents">2</span>3</div>');
+  is(pasteCount, 1, "paste event was not triggered");
+
   // test that we can preventDefault pastes
   pasteFunc = function (event) { event.preventDefault(); return false; };
   await copyToClipBoard("<pre>Kitty</pre><span>Hello</span>", true);
   trans = getTransferableFromClipboard(true);
   pasteInto(trans, '<pre id="paste_here">Hello </pre>',"paste_here");
   verifyContent('<pre id="paste_here">Hello </pre>');
   is(pasteCount, 0, "paste event was triggered");
 }
--- a/layout/base/nsCSSFrameConstructor.cpp
+++ b/layout/base/nsCSSFrameConstructor.cpp
@@ -443,17 +443,17 @@ IsInlineFrame(const nsIFrame* aFrame)
 }
 
 /**
  * True for display: contents elements.
  */
 static inline bool
 IsDisplayContents(const Element* aElement)
 {
-  return aElement->HasServoData() && Servo_Element_IsDisplayContents(aElement);
+  return aElement->IsDisplayContents();
 }
 
 static inline bool
 IsDisplayContents(const nsIContent* aContent)
 {
   return aContent->IsElement() && IsDisplayContents(aContent->AsElement());
 }
 
--- a/layout/generic/nsFrameSelection.cpp
+++ b/layout/generic/nsFrameSelection.cpp
@@ -1586,18 +1586,17 @@ nsFrameSelection::RepaintSelection(Selec
   }
 #endif
   return mDomSelections[index]->Repaint(mShell->GetPresContext());
 }
 
 static bool
 IsDisplayContents(const nsIContent* aContent)
 {
-  return aContent->IsElement() && aContent->AsElement()->HasServoData() &&
-    Servo_Element_IsDisplayContents(aContent->AsElement());
+  return aContent->IsElement() && aContent->AsElement()->IsDisplayContents();
 }
 
 nsIFrame*
 nsFrameSelection::GetFrameForNodeOffset(nsIContent*        aNode,
                                         int32_t            aOffset,
                                         CaretAssociateHint aHint,
                                         int32_t*           aReturnOffset) const
 {