Bug 1457026: Teach nsDocumentEncoder about display: contents. r=mats
authorEmilio Cobos Álvarez <emilio@crisal.io>
Thu, 26 Apr 2018 02:14:08 +0200
changeset 471926 422aebe40856002e8e53916433703840dc6fafa9
parent 471925 28c2803c243aaccac97653e21013450e74f4ef90
child 471927 46864f33859eecba12b01b4011d392ee20520648
push id1728
push userjlund@mozilla.com
push dateMon, 18 Jun 2018 21:12:27 +0000
treeherdermozilla-release@c296fde26f5f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmats
bugs1457026
milestone61.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
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
 {