After copy and paste, cursor may appear to be above the bottom of content
authormmaxfield@apple.com <mmaxfield@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 22 Feb 2014 03:14:13 +0000 (03:14 +0000)
committermmaxfield@apple.com <mmaxfield@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 22 Feb 2014 03:14:13 +0000 (03:14 +0000)
https://bugs.webkit.org/show_bug.cgi?id=129167

Reviewed by Ryosuke Niwa.

Source/WebCore:

Adding a clear:both to the end of content.

I can't handle the case of the cursor appearing above the bottom of
absolutely positioned divs (of the case of floats inside absolutely
positioned divs) because you can't know where the bottom of a div
will end up being rendered (it is affected by things like browser
window width and text size settings). Therefore, the only case I
can handle is the case where there is a floating div in the same
level as the document itself.

Test: editing/pasteboard/copy-paste-inserts-clearing-div.html

* editing/EditingStyle.cpp:
(WebCore::EditingStyle::isFloating):
* editing/EditingStyle.h:
* editing/markup.cpp:
(WebCore::StyledMarkupAccumulator::StyledMarkupAccumulator):
(WebCore::StyledMarkupAccumulator::appendElement):
(WebCore::createMarkupInternal):

LayoutTests:

Makes sure that the clearing div is inserted.

* editing/pasteboard/copy-paste-inserts-clearing-div-expected.txt: Added.
* editing/pasteboard/copy-paste-inserts-clearing-div.html: Added.

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

LayoutTests/ChangeLog
LayoutTests/editing/pasteboard/copy-paste-inserts-clearing-div-expected.txt [new file with mode: 0644]
LayoutTests/editing/pasteboard/copy-paste-inserts-clearing-div.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/editing/EditingStyle.cpp
Source/WebCore/editing/EditingStyle.h
Source/WebCore/editing/markup.cpp

index 69ec54460da7daa08c3fc9c265915e9421da2915..6890b347aa2ca2c6da65ace46d8c526fb73b75ca 100644 (file)
@@ -1,3 +1,15 @@
+2014-02-21  Myles C. Maxfield  <mmaxfield@apple.com>
+
+        After copy and paste, cursor may appear to be above the bottom of content
+        https://bugs.webkit.org/show_bug.cgi?id=129167
+
+        Reviewed by Ryosuke Niwa.
+
+        Makes sure that the clearing div is inserted.
+
+        * editing/pasteboard/copy-paste-inserts-clearing-div-expected.txt: Added.
+        * editing/pasteboard/copy-paste-inserts-clearing-div.html: Added.
+
 2014-02-21  Brian Burg  <bburg@apple.com>
 
         Move unported Web Inspector tests to LayoutTests/inspector-obsolete
diff --git a/LayoutTests/editing/pasteboard/copy-paste-inserts-clearing-div-expected.txt b/LayoutTests/editing/pasteboard/copy-paste-inserts-clearing-div-expected.txt
new file mode 100644 (file)
index 0000000..fe02214
--- /dev/null
@@ -0,0 +1,100 @@
+This tests to see if floating elements cause a clearing element to be inserted upon copy/paste
+
+first test - before:
+| <html>
+|   <head>
+|     "
+"
+|     <meta>
+|       content="text/html; charset=utf-8"
+|       http-equiv="Content-type"
+|     "
+"
+|     <script>
+|       src="../../resources/dump-as-markup.js"
+|       type="text/javascript"
+|     "
+"
+|   "
+"
+|   <body>
+|     "
+Before
+"
+|     <div>
+|       style="position: absolute; top: 0px; right: 0px; width: 100px; height: 100px; background: yellow;"
+|     "
+"
+|     <div>
+|       style="float: right; width: 200px; height: 200px; background: blue;"
+|     "
+After
+
+"
+|     <script>
+|       "
+
+Markup.description('This tests to see if floating elements cause a clearing element to be inserted upon copy/paste');
+
+document.designMode = 'on';
+
+if (window.internals)
+    window.internals.settings.setShouldConvertPositionStyleOnCopy(true);
+
+var s = window.getSelection();
+
+Markup.dump('test1', 'first test - before');
+document.execCommand("SelectAll");
+document.execCommand("Cut");
+document.execCommand("Paste");
+Markup.dump('test1', 'first test - after');
+
+"
+
+first test - after:
+| <html>
+|   <head>
+|     "
+"
+|     <meta>
+|       content="text/html; charset=utf-8"
+|       http-equiv="Content-type"
+|     "
+"
+|     <script>
+|       src="../../resources/dump-as-markup.js"
+|       type="text/javascript"
+|     "
+"
+|   "
+"
+|   <body>
+|     <div>
+|       style="position: relative;"
+|       "BeforeĀ "
+|       <div>
+|         style="position: absolute; top: 0px; right: 0px; width: 100px; height: 100px; background-color: yellow;"
+|       <div>
+|         style="float: right; width: 200px; height: 200px; background-color: blue;"
+|       "After<#selection-caret>"
+|       <div>
+|         style="clear: both;"
+|     <script>
+|       "
+
+Markup.description('This tests to see if floating elements cause a clearing element to be inserted upon copy/paste');
+
+document.designMode = 'on';
+
+if (window.internals)
+    window.internals.settings.setShouldConvertPositionStyleOnCopy(true);
+
+var s = window.getSelection();
+
+Markup.dump('test1', 'first test - before');
+document.execCommand("SelectAll");
+document.execCommand("Cut");
+document.execCommand("Paste");
+Markup.dump('test1', 'first test - after');
+
+"
diff --git a/LayoutTests/editing/pasteboard/copy-paste-inserts-clearing-div.html b/LayoutTests/editing/pasteboard/copy-paste-inserts-clearing-div.html
new file mode 100644 (file)
index 0000000..1b561cf
--- /dev/null
@@ -0,0 +1,31 @@
+<html>
+<head>
+<meta http-equiv="Content-type" content="text/html; charset=utf-8" />
+<script type="text/javascript" src="../../resources/dump-as-markup.js"></script>
+</head>
+<body>
+Before
+<div style="position: absolute; top: 0px; right: 0px; width: 100px; height: 100px; background: yellow;"></div>
+<div style="float: right; width: 200px; height: 200px; background: blue;"></div>
+After
+
+<script>
+
+Markup.description('This tests to see if floating elements cause a clearing element to be inserted upon copy/paste');
+
+document.designMode = 'on';
+
+if (window.internals)
+    window.internals.settings.setShouldConvertPositionStyleOnCopy(true);
+
+var s = window.getSelection();
+
+Markup.dump('test1', 'first test - before');
+document.execCommand("SelectAll");
+document.execCommand("Cut");
+document.execCommand("Paste");
+Markup.dump('test1', 'first test - after');
+
+</script>
+</body>
+</html>
index 122dda7c8084be8c9f444ab0a495adf9ad8c111c..e42a1d0ee563d5230b654c501a23f19486655fb4 100644 (file)
@@ -1,3 +1,30 @@
+2014-02-21  Myles C. Maxfield  <mmaxfield@apple.com>
+
+        After copy and paste, cursor may appear to be above the bottom of content
+        https://bugs.webkit.org/show_bug.cgi?id=129167
+
+        Reviewed by Ryosuke Niwa.
+
+        Adding a clear:both to the end of content.
+
+        I can't handle the case of the cursor appearing above the bottom of
+        absolutely positioned divs (of the case of floats inside absolutely
+        positioned divs) because you can't know where the bottom of a div
+        will end up being rendered (it is affected by things like browser
+        window width and text size settings). Therefore, the only case I
+        can handle is the case where there is a floating div in the same
+        level as the document itself.
+
+        Test: editing/pasteboard/copy-paste-inserts-clearing-div.html
+
+        * editing/EditingStyle.cpp:
+        (WebCore::EditingStyle::isFloating):
+        * editing/EditingStyle.h:
+        * editing/markup.cpp:
+        (WebCore::StyledMarkupAccumulator::StyledMarkupAccumulator):
+        (WebCore::StyledMarkupAccumulator::appendElement):
+        (WebCore::createMarkupInternal):
+
 2014-02-21  Dean Jackson  <dino@apple.com>
 
         [iOS Media] Wireless target UI
index 916369dcd1d889f464a8abf885790f87524c425e..424a06f0040accc8a1741272aba57356ca3ef9df 100644 (file)
@@ -1242,6 +1242,13 @@ bool EditingStyle::convertPositionStyle()
     return false;
 }
 
+bool EditingStyle::isFloating()
+{
+    RefPtr<CSSValue> v = m_mutableStyle->getPropertyCSSValue(CSSPropertyFloat);
+    RefPtr<CSSPrimitiveValue> noneValue = cssValuePool().createIdentifierValue(CSSValueNone);
+    return v && !v->equals(*noneValue);
+}
+
 int EditingStyle::legacyFontSize(Document* document) const
 {
     RefPtr<CSSValue> cssValue = m_mutableStyle->getPropertyCSSValue(CSSPropertyFontSize);
index 23e3be05a74d4854af74275d6130541413a9a3a5..8200903795fc915979d3aca9a5c1d788c998e804 100644 (file)
@@ -136,6 +136,7 @@ public:
     void removePropertiesInElementDefaultStyle(Element*);
     void forceInline();
     bool convertPositionStyle();
+    bool isFloating();
     int legacyFontSize(Document*) const;
 
     float fontSizeDelta() const { return m_fontSizeDelta; }
index 3b51f5134c6c3ff1bfa437b96f627ed16ea30c2c..1dd344787b4ccdd2dc8d19a27a6a66f60e923a07 100644 (file)
@@ -125,6 +125,7 @@ public:
     String takeResults();
     
     bool needRelativeStyleWrapper() const { return m_needRelativeStyleWrapper; }
+    bool needClearingDiv() const { return m_needClearingDiv; }
 
     using MarkupAccumulator::appendString;
 
@@ -162,6 +163,7 @@ private:
     RefPtr<EditingStyle> m_wrappingStyle;
     bool m_needRelativeStyleWrapper;
     bool m_needsPositionStyleConversion;
+    bool m_needClearingDiv;
 };
 
 inline StyledMarkupAccumulator::StyledMarkupAccumulator(Vector<Node*>* nodes, EAbsoluteURLs shouldResolveURLs, EAnnotateForInterchange shouldAnnotate, const Range* range, bool needsPositionStyleConversion, Node* highestNodeToBeSerialized)
@@ -170,6 +172,7 @@ inline StyledMarkupAccumulator::StyledMarkupAccumulator(Vector<Node*>* nodes, EA
     , m_highestNodeToBeSerialized(highestNodeToBeSerialized)
     , m_needRelativeStyleWrapper(false)
     , m_needsPositionStyleConversion(needsPositionStyleConversion)
+    , m_needClearingDiv(false)
 {
 }
 
@@ -327,8 +330,10 @@ void StyledMarkupAccumulator::appendElement(StringBuilder& out, const Element& e
             if (addDisplayInline)
                 newInlineStyle->forceInline();
             
-            if (m_needsPositionStyleConversion)
+            if (m_needsPositionStyleConversion) {
                 m_needRelativeStyleWrapper |= newInlineStyle->convertPositionStyle();
+                m_needClearingDiv |= newInlineStyle->isFloating();
+            }
 
             // If the node is not fully selected by the range, then we don't want to keep styles that affect its relationship to the nodes around it
             // only the ones that affect it and the nodes within it.
@@ -629,6 +634,8 @@ static String createMarkupInternal(Document& document, const Range& range, const
     }
     
     if (accumulator.needRelativeStyleWrapper() && needsPositionStyleConversion) {
+        if (accumulator.needClearingDiv())
+            accumulator.appendString("<div style=\"clear: both;\"></div>");
         RefPtr<EditingStyle> positionRelativeStyle = styleFromMatchedRulesAndInlineDecl(body);
         positionRelativeStyle->style()->setProperty(CSSPropertyPosition, CSSValueRelative);
         accumulator.wrapWithStyleNode(positionRelativeStyle->style(), document, true);