Changeset 153509 in webkit

Timestamp:

Jul 30, 2013 9:55:02 PM (11 years ago)

Author:

commit-queue@webkit.org

Message:

Gmail reply email - Bold and Italic style get stuck
​https://bugs.webkit.org/show_bug.cgi?id=118185

Patch by Vani Hegde <​vani.hegde@samsung.com> on 2013-07-30
Reviewed by Ryosuke Niwa.

Source/WebCore:

When bold/italic style is applied to a content with mixed editability,
style is applied appropriately for the first time.
But after this, style toggling does not work any more. Same behavior
can be observed when the content on which style is applied contains
text node without renderer (Ex: text node corresponding to tab/space etc)

When bold/italic style is applied on a selection, we actually apply the
style only for text nodes that have renderers and also are contenteditable.
Similarly during style toggling, to check whether a style has been already
applied on a selection

Tests: editing/style/toggle-style-bold-italic-mixed-editability.html

editing/style/toggle-style-bold-italic.html

• editing/EditingStyle.cpp:

(WebCore::EditingStyle::triStateOfStyle):
A particular selection on which style is applied may contain text nodes
without renderers(text nodes corresponding to tab/space) or text nodes
that are not contenteditable. We do not apply style to such text nodes.
Hence, even during style toggling we should not consider the styles
present in such nodes.

LayoutTests:

Layout tests added to test bold/italic style toggling on a content
with mixed editability.

• editing/editing.js:

(runDumpAsTextEditingTest): Replaced markupResultList with
elementsForDumpingMarkupList to store all markup data to be logged
at the end of test.
(debugForDumpAsText): Changed to use elementsForDumpingMarkupList.
(startNewMarkupGroup): Added utility function to hold intermediate
data by separating <ol>.

• editing/style/toggle-style-bold-italic-expected.txt: Added.
• editing/style/toggle-style-bold-italic-mixed-editability-expected.txt: Added.
• editing/style/toggle-style-bold-italic-mixed-editability.html: Added.
• editing/style/toggle-style-bold-italic.html: Added.

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/editing/editing.js (modified) (3 diffs)
• LayoutTests/editing/style/toggle-style-bold-italic-expected.txt (added)
• LayoutTests/editing/style/toggle-style-bold-italic-mixed-editability-expected.txt (added)
• LayoutTests/editing/style/toggle-style-bold-italic-mixed-editability.html (added)
• LayoutTests/editing/style/toggle-style-bold-italic.html (added)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/editing/EditingStyle.cpp (modified) (1 diff)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2013-07-30  Vani Hegde  <vani.hegde@samsung.com>
+
+        Gmail reply email - Bold and Italic style  get stuck
+        https://bugs.webkit.org/show_bug.cgi?id=118185
+
+        Reviewed by Ryosuke Niwa.
+
+        Layout tests added to test bold/italic style toggling on a content
+        with mixed editability.
+
+        * editing/editing.js:
+        (runDumpAsTextEditingTest): Replaced markupResultList with
+        elementsForDumpingMarkupList to store all markup data to be logged
+        at the end of test.
+        (debugForDumpAsText): Changed to use elementsForDumpingMarkupList.
+        (startNewMarkupGroup): Added utility function to hold intermediate
+        data by separating <ol>.
+        * editing/style/toggle-style-bold-italic-expected.txt: Added.
+        * editing/style/toggle-style-bold-italic-mixed-editability-expected.txt: Added.
+        * editing/style/toggle-style-bold-italic-mixed-editability.html: Added.
+        * editing/style/toggle-style-bold-italic.html: Added.
+
 2013-07-30  Rob Buis  <rwlbuis@webkit.org>
 

trunk/LayoutTests/editing/editing.js

@@ -905 +905 @@
 
 var dumpAsText = false;
-var markupResultList = document.createElement('ol');
+var elementsForDumpingMarkupList = [document.createElement('ol')];
 
 function runDumpAsTextEditingTest(enableCallbacks) {
@@ -921 +921 @@
     editingTest();
 
-    document.body.appendChild(markupResultList);
+    for (var i = 0; i < elementsForDumpingMarkupList.length; i++)
+        document.body.appendChild(elementsForDumpingMarkupList[i]);
 }
 
@@ -928 +929 @@
         var newItem = document.createElement('li');
         newItem.appendChild(document.createTextNode(name+": "+document.getElementById("root").innerHTML));
-        markupResultList.appendChild(newItem);
-    }
+        elementsForDumpingMarkupList[elementsForDumpingMarkupList.length - 1].appendChild(newItem);
+    }
+}
+
+function startNewMarkupGroup(label) {
+    if (!elementsForDumpingMarkupList[elementsForDumpingMarkupList.length - 1].hasChildNodes())
+        elementsForDumpingMarkupList.pop();
+    elementsForDumpingMarkupList.push(document.createElement('br'));
+    elementsForDumpingMarkupList.push(document.createTextNode(label));
+    elementsForDumpingMarkupList.push(document.createElement('ol'));
 }
 

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2013-07-30  Vani Hegde  <vani.hegde@samsung.com>
+
+        Gmail reply email - Bold and Italic style  get stuck
+        https://bugs.webkit.org/show_bug.cgi?id=118185
+
+        Reviewed by Ryosuke Niwa.
+
+        When bold/italic style is applied to a content with mixed editability,
+        style is applied appropriately for the first time.
+        But after this, style toggling does not work any more. Same behavior
+        can be observed when the content on which style is applied contains
+        text node without renderer (Ex: text node corresponding to tab/space etc)
+
+        When bold/italic style is applied on a selection, we actually apply the
+        style only for text nodes that have renderers and also are contenteditable.
+        Similarly during style toggling, to check whether a style has been already
+        applied on a selection
+
+        Tests: editing/style/toggle-style-bold-italic-mixed-editability.html
+               editing/style/toggle-style-bold-italic.html
+
+        * editing/EditingStyle.cpp:
+        (WebCore::EditingStyle::triStateOfStyle):
+        A particular selection on which style is applied may contain text nodes
+        without renderers(text nodes corresponding to tab/space) or text nodes
+        that are not contenteditable. We do not apply style to such text nodes.
+        Hence, even during style toggling we should not consider the styles
+        present in such nodes.
+
 2013-07-30  Rob Buis  <rwlbuis@webkit.org>
 

trunk/Source/WebCore/editing/EditingStyle.cpp

@@ -684 +684 @@
 
     TriState state = FalseTriState;
+    bool nodeIsStart = true;
     for (Node* node = selection.start().deprecatedNode(); node; node = NodeTraversal::next(node)) {
-        ComputedStyleExtractor computedStyle(node);
-        TriState nodeState = triStateOfStyle(&computedStyle, node->isTextNode() ? EditingStyle::DoNotIgnoreTextOnlyProperties : EditingStyle::IgnoreTextOnlyProperties);
-        if (node == selection.start().deprecatedNode())
-            state = nodeState;
-        else if (state != nodeState && node->isTextNode()) {
-            state = MixedTriState;
-            break;
+        if (node->renderer() && node->rendererIsEditable()) {
+            ComputedStyleExtractor computedStyle(node);
+            TriState nodeState = triStateOfStyle(&computedStyle, node->isTextNode() ? EditingStyle::DoNotIgnoreTextOnlyProperties : EditingStyle::IgnoreTextOnlyProperties);
+            if (nodeIsStart) {
+                state = nodeState;
+                nodeIsStart = false;
+            } else if (state != nodeState && node->isTextNode()) {
+                state = MixedTriState;
+                break;
+            }
         }
+
         if (node == selection.end().deprecatedNode())
             break;