Attachment 147558 Details for Bug 34608 – Proposed Patch

[patch] Proposed Patch

bug-34608-20120614230754.patch (text/plain), 14.82 KB, created by Pravin D on 2012-06-14 05:08:33 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Pravin D

Created: 2012-06-14 05:08:33 PDT

Size: 14.82 KB

patch

obsolete

>Index: Source/WebCore/ChangeLog
>===================================================================
>--- Source/WebCore/ChangeLog	(revision 120313)
>+++ Source/WebCore/ChangeLog	(working copy)
>@@ -1,3 +1,31 @@
>+2012-06-14  Pravin D  <pravind.2k4@gmail.com>
>+
>+        Deleting text leaves the style of text in the typing style
>+        https://bugs.webkit.org/show_bug.cgi?id=34608
>+
>+        While deleting typing style is not updated if the seletion is within a text node. However if are doing a series of deletes which
>+        involves deleting some nodes and results in the current selection for this delete operation to within a text node, then the typing style
>+        set in FrameSeletion class might be stale and has to be updated. 
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Test: editing/deleting/delete-reset-typing-style.html
>+
>+        * editing/DeleteSelectionCommand.cpp:
>+        (WebCore::DeleteSelectionCommand::saveTypingStyleState):
>+        Create new typing style if the flag(m_updateTypingStyle) set in FrameSeletion class irrespective 
>+        of the fact that we are deleting text from the same text node.
>+
>+        (WebCore::DeleteSelectionCommand::removeNode):
>+        Set flag(m_updateTypingStyle) in FrameSeletion to update typing style during the next delete operation.
>+
>+        * editing/FrameSelection.h:
>+        (WebCore::FrameSelection::shouldUpdateTypingStyle):
>+        (WebCore::FrameSelection::setUpdateTypingStyle):
>+        Getter and Setter functions for m_updateTypingStyle.
>+
>+        (FrameSelection):
>+
> 2012-06-14  Yoshifumi Inoue  <yosin@chromium.org>
> 
>         [Form] Replace InputNumber type to Decimal type and drop InputNumber
>Index: Source/WebCore/editing/DeleteSelectionCommand.cpp
>===================================================================
>--- Source/WebCore/editing/DeleteSelectionCommand.cpp	(revision 120296)
>+++ Source/WebCore/editing/DeleteSelectionCommand.cpp	(working copy)
>@@ -280,7 +280,8 @@ void DeleteSelectionCommand::saveTypingS
>     // typing style at the start of the selection, nor is there a reason to 
>     // compute the style at the start of the selection after deletion (see the 
>     // early return in calculateTypingStyleAfterDelete).
>-    if (m_upstreamStart.deprecatedNode() == m_downstreamEnd.deprecatedNode() && m_upstreamStart.deprecatedNode()->isTextNode())
>+    if (m_upstreamStart.deprecatedNode() == m_downstreamEnd.deprecatedNode() && m_upstreamStart.deprecatedNode()->isTextNode()
>+        && !document()->frame()->selection()->shouldUpdateTypingStyle())
>         return;
> 
>     // Figure out the typing style in effect before the delete is done.
>@@ -293,6 +294,7 @@ void DeleteSelectionCommand::saveTypingS
>         m_deleteIntoBlockquoteStyle = EditingStyle::create(m_selectionToDelete.end());
>     else
>         m_deleteIntoBlockquoteStyle = 0;
>+    document()->frame()->selection()->setUpdateTypingStyle(false);
> }
> 
> bool DeleteSelectionCommand::handleSpecialCaseBRDelete()
>@@ -384,6 +386,8 @@ void DeleteSelectionCommand::removeNode(
>     updatePositionForNodeRemoval(m_trailingWhitespace, node.get());
>     
>     CompositeEditCommand::removeNode(node);
>+
>+    document()->frame()->selection()->setUpdateTypingStyle(true);
> }
> 
> static void updatePositionForTextRemoval(Node* node, int offset, int count, Position& position)
>Index: Source/WebCore/editing/FrameSelection.h
>===================================================================
>--- Source/WebCore/editing/FrameSelection.h	(revision 120296)
>+++ Source/WebCore/editing/FrameSelection.h	(working copy)
>@@ -239,6 +239,9 @@ public:
>     void setTypingStyle(PassRefPtr<EditingStyle>);
>     void clearTypingStyle();
> 
>+    bool shouldUpdateTypingStyle() { return m_updateTypingStyle; }
>+    void setUpdateTypingStyle(bool v) { m_updateTypingStyle = v; }
>+
>     FloatRect bounds(bool clipToVisibleContent = true) const;
> 
>     void getClippedVisibleTextRectangles(Vector<FloatRect>&) const;
>@@ -303,6 +306,7 @@ private:
>     bool m_caretPaint : 1;
>     bool m_isCaretBlinkingSuspended : 1;
>     bool m_focused : 1;
>+    bool m_updateTypingStyle : 1;
> };
> 
> inline EditingStyle* FrameSelection::typingStyle() const
>Index: LayoutTests/ChangeLog
>===================================================================
>--- LayoutTests/ChangeLog	(revision 120313)
>+++ LayoutTests/ChangeLog	(working copy)
>@@ -1,3 +1,26 @@
>+2012-06-14  Pravin D  <pravind.2k4@gmail.com>
>+
>+        Deleting text leaves the style of text in the typing style
>+        https://bugs.webkit.org/show_bug.cgi?id=34608
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        * editing/deleting/delete-reset-typing-style-expected.txt: Added.
>+        * editing/deleting/delete-reset-typing-style.html: Added.
>+         Test case for the bug.
>+
>+        * editing/style/5084241.html:
>+         Converted testcase to use Markup.dump().
>+
>+        * editing/style/5084241-expected.txt: Added.
>+        New expected file containing the Markup.dump().
>+
>+        * platform/chromium-win/editing/style/5084241-expected.txt: Removed.
>+        * platform/gtk/editing/style/5084241-expected.txt: Removed.
>+        * platform/mac/editing/style/5084241-expected.txt: Removed.
>+        * platform/qt/editing/style/5084241-expected.txt: Removed.
>+        Old expected files containing render tree dump.
>+
> 2012-06-14  Jan Keromnes  <janx@linux.com>
> 
>         Web Inspector: Implement ExtensionPanel.show() method
>Index: LayoutTests/editing/deleting/delete-reset-typing-style-expected.txt
>===================================================================
>--- LayoutTests/editing/deleting/delete-reset-typing-style-expected.txt	(revision 0)
>+++ LayoutTests/editing/deleting/delete-reset-typing-style-expected.txt	(revision 0)
>@@ -0,0 +1,30 @@
>+
>+When the text is deleted from a editable container, the typing styles like Bold, Italic , underline must be reset appropriately.
>+In the following testcase non of the text must have any styles like bold or italic. All must be plain text.
>+
>+| "
>+"
>+| <div>
>+|   id="bold"
>+|   "
>+Filler "
>+|   <b>
>+|     "This should not be BOLD"
>+| "
>+"
>+| <div>
>+|   id="italic"
>+|   "
>+Filler "
>+|   <i>
>+|     "This should not be italic"
>+| "
>+"
>+| <div>
>+|   id="underline"
>+|   "
>+Filler "
>+|   <u>
>+|     "This should not be contain underline<#selection-caret>"
>+| "
>+"
>Index: LayoutTests/editing/deleting/delete-reset-typing-style.html
>===================================================================
>--- LayoutTests/editing/deleting/delete-reset-typing-style.html	(revision 0)
>+++ LayoutTests/editing/deleting/delete-reset-typing-style.html	(revision 0)
>@@ -0,0 +1,50 @@
>+<title> Test case for bug https://bugs.webkit.org/show_bug.cgi?id=34608</title>
>+<script src="../../resources/dump-as-markup.js"></script>
>+<body>
>+<p id="description">
>+When the text is deleted from a editable container, the typing styles like Bold, Italic , underline must be reset appropriately.
>+In the following testcase non of the text must have any styles like bold or italic. All must be plain text.
>+</p>
>+<div id="test" contenteditable>
>+<div id="bold">
>+Filler <b>Bold</b> Filler
>+</div>
>+<div id="italic">
>+Filler <i>Italic</i> Filler
>+</div>
>+<div id="underline">
>+Filler <u>Underline</u> Filler
>+</div>
>+</div>
>+<script>
>+function deleteBold() {
>+    var test = document.getElementById('bold');
>+    window.getSelection().setPosition(test, test.innerHTML.length);
>+    for (var i = 0; i < 11; i++)
>+        document.execCommand('Delete', false, null);
>+    document.execCommand('InsertText', false, 'This should not be BOLD');
>+}
>+
>+function deleteItalic() {
>+    var test = document.getElementById('italic');
>+    window.getSelection().setPosition(test, test.innerHTML.length);
>+    for (var i = 0; i < 13; i++)
>+        document.execCommand('Delete', false, null);
>+    document.execCommand('InsertText', false, 'This should not be italic');
>+}
>+
>+function deleteUnderline() {
>+    var test = document.getElementById('underline');
>+    window.getSelection().setPosition(test, test.innerHTML.length);
>+    for (var i = 0; i < 16; i++)
>+        document.execCommand('Delete', false, null);
>+    document.execCommand('InsertText', false, 'This should not be contain underline');
>+}
>+
>+Markup.description(document.getElementById('description').textContent);
>+deleteBold();
>+deleteItalic();
>+deleteUnderline();
>+Markup.dump("test");
>+</script>
>+</body>
>Index: LayoutTests/editing/style/5084241-expected.txt
>===================================================================
>--- LayoutTests/editing/style/5084241-expected.txt	(revision 0)
>+++ LayoutTests/editing/style/5084241-expected.txt	(revision 0)
>@@ -0,0 +1,7 @@
>+This tests for a bug where deleting everything in a paragraph of colored text would prevent the user from changing the text color.
>+| <font>
>+|   color="#ff0000"
>+|   "This text should be red."
>+| <font>
>+|   color="#0000ff"
>+|   "This text should be blue.<#selection-caret>"
>Index: LayoutTests/editing/style/5084241.html
>===================================================================
>--- LayoutTests/editing/style/5084241.html	(revision 120296)
>+++ LayoutTests/editing/style/5084241.html	(working copy)
>@@ -1,4 +1,5 @@
>-<p>This tests for a bug where deleting everything in a paragraph of colored text would prevent the user from changing the text color.</p>
>+<script src="../../resources/dump-as-markup.js"></script>
>+<p id="description">This tests for a bug where deleting everything in a paragraph of colored text would prevent the user from changing the text color.</p>
> <div id="div" contenteditable="true"><br></div>
> 
> <script>
>@@ -12,4 +13,6 @@ document.execCommand("Delete");
> document.execCommand("InsertText", false, "This text should be red.");
> document.execCommand("ForeColor", false, "blue");
> document.execCommand("InsertText", false, "This text should be blue.");
>+Markup.description(document.getElementById('description').textContent);
>+Markup.dump("div");
> </script>
>Index: LayoutTests/platform/chromium-win/editing/style/5084241-expected.txt
>===================================================================
>--- LayoutTests/platform/chromium-win/editing/style/5084241-expected.txt	(revision 120296)
>+++ LayoutTests/platform/chromium-win/editing/style/5084241-expected.txt	(working copy)
>@@ -1,16 +0,0 @@
>-layer at (0,0) size 800x600
>-  RenderView at (0,0) size 800x600
>-layer at (0,0) size 800x600
>-  RenderBlock {HTML} at (0,0) size 800x600
>-    RenderBody {BODY} at (8,8) size 784x584
>-      RenderBlock {P} at (0,0) size 784x20
>-        RenderText {#text} at (0,0) size 768x19
>-          text run at (0,0) width 768: "This tests for a bug where deleting everything in a paragraph of colored text would prevent the user from changing the text color."
>-      RenderBlock {DIV} at (0,36) size 784x20
>-        RenderInline {FONT} at (0,0) size 141x19 [color=#FF0000]
>-          RenderText {#text} at (0,0) size 141x19
>-            text run at (0,0) width 141: "This text should be red."
>-        RenderInline {FONT} at (0,0) size 146x19 [color=#0000FF]
>-          RenderText {#text} at (141,0) size 146x19
>-            text run at (141,0) width 146: "This text should be blue."
>-caret: position 25 of child 0 {#text} of child 1 {FONT} of child 2 {DIV} of body
>Index: LayoutTests/platform/gtk/editing/style/5084241-expected.txt
>===================================================================
>--- LayoutTests/platform/gtk/editing/style/5084241-expected.txt	(revision 120296)
>+++ LayoutTests/platform/gtk/editing/style/5084241-expected.txt	(working copy)
>@@ -1,17 +0,0 @@
>-layer at (0,0) size 800x600
>-  RenderView at (0,0) size 800x600
>-layer at (0,0) size 800x600
>-  RenderBlock {HTML} at (0,0) size 800x600
>-    RenderBody {BODY} at (8,8) size 784x584
>-      RenderBlock {P} at (0,0) size 784x36
>-        RenderText {#text} at (0,0) size 771x37
>-          text run at (0,0) width 771: "This tests for a bug where deleting everything in a paragraph of colored text would prevent the user from changing the text"
>-          text run at (0,18) width 36: "color."
>-      RenderBlock {DIV} at (0,52) size 784x18
>-        RenderInline {FONT} at (0,0) size 148x19 [color=#FF0000]
>-          RenderText {#text} at (0,0) size 148x19
>-            text run at (0,0) width 148: "This text should be red."
>-        RenderInline {FONT} at (0,0) size 155x19 [color=#0000FF]
>-          RenderText {#text} at (148,0) size 155x19
>-            text run at (148,0) width 155: "This text should be blue."
>-caret: position 25 of child 0 {#text} of child 1 {FONT} of child 2 {DIV} of body
>Index: LayoutTests/platform/mac/editing/style/5084241-expected.txt
>===================================================================
>--- LayoutTests/platform/mac/editing/style/5084241-expected.txt	(revision 120296)
>+++ LayoutTests/platform/mac/editing/style/5084241-expected.txt	(working copy)
>@@ -1,17 +0,0 @@
>-layer at (0,0) size 800x600
>-  RenderView at (0,0) size 800x600
>-layer at (0,0) size 800x600
>-  RenderBlock {HTML} at (0,0) size 800x600
>-    RenderBody {BODY} at (8,8) size 784x584
>-      RenderBlock {P} at (0,0) size 784x36
>-        RenderText {#text} at (0,0) size 771x36
>-          text run at (0,0) width 771: "This tests for a bug where deleting everything in a paragraph of colored text would prevent the user from changing the text"
>-          text run at (0,18) width 36: "color."
>-      RenderBlock {DIV} at (0,52) size 784x18
>-        RenderInline {FONT} at (0,0) size 148x18 [color=#FF0000]
>-          RenderText {#text} at (0,0) size 148x18
>-            text run at (0,0) width 148: "This text should be red."
>-        RenderInline {FONT} at (0,0) size 155x18 [color=#0000FF]
>-          RenderText {#text} at (148,0) size 155x18
>-            text run at (148,0) width 155: "This text should be blue."
>-caret: position 25 of child 0 {#text} of child 1 {FONT} of child 2 {DIV} of body
>Index: LayoutTests/platform/qt/editing/style/5084241-expected.txt
>===================================================================
>--- LayoutTests/platform/qt/editing/style/5084241-expected.txt	(revision 120296)
>+++ LayoutTests/platform/qt/editing/style/5084241-expected.txt	(working copy)
>@@ -1,17 +0,0 @@
>-layer at (0,0) size 800x600
>-  RenderView at (0,0) size 800x600
>-layer at (0,0) size 800x600
>-  RenderBlock {HTML} at (0,0) size 800x600
>-    RenderBody {BODY} at (8,8) size 784x584
>-      RenderBlock {P} at (0,0) size 784x42
>-        RenderText {#text} at (0,0) size 731x42
>-          text run at (0,0) width 731: "This tests for a bug where deleting everything in a paragraph of colored text would prevent the user from"
>-          text run at (0,21) width 164: "changing the text color."
>-      RenderBlock {DIV} at (0,58) size 784x21
>-        RenderInline {FONT} at (0,0) size 165x21 [color=#FF0000]
>-          RenderText {#text} at (0,0) size 165x21
>-            text run at (0,0) width 165: "This text should be red."
>-        RenderInline {FONT} at (0,0) size 173x21 [color=#0000FF]
>-          RenderText {#text} at (165,0) size 173x21
>-            text run at (165,0) width 173: "This text should be blue."
>-caret: position 25 of child 0 {#text} of child 1 {FONT} of child 2 {DIV} of body

Actions: View | Formatted Diff | Diff

Attachments on bug 34608: 72151 | 105614 | 146379 | 146685 | 147558 | 148162 | 148272