Changeset 82791 in webkit


Ignore:
Timestamp:
Apr 3, 2011 12:54:33 AM (13 years ago)
Author:
rniwa@webkit.org
Message:

2011-03-21 Ryosuke Niwa <​rniwa@webkit.org>

Reviewed by Eric Seidel.

editing commands shouldn't run when there's no body
​https://bugs.webkit.org/show_bug.cgi?id=56771

The bug was caused by WebKit's not checking the existence of root editable element
in enabled* functions. Although isContentEditable returns true whenever we're in design mode,
we should not run editing commands in a document without a body element editable because
doing so results in appending a non-body element to the document node.

Fixed the bug by modifying various enabled* functions to ensure we have a root editable element.
New behavior tries to match that of Firefox except StyleWithCSS, which Firefox seems to ignore
when there are no body element. Since StyleWithCSS is a document's state or property, we allow
execCommand('StyleWithCSS') even in a document without a body element.

WebKit's and Firefox's behaviors also deviate in insert-image-with-selecting-document.html.
Whereas WebKit respects selection set by script and ignores execCommand, Firefox modifies
the selection when document.write("x") is ran and successfully inserts image.

Thus, empty-document-delete.html and empty-document-justify-right.html both pass on Firefox
while empty-document-stylewithcss.html and insert-image-with-selecting-document.html both fail.

Since Internet Explorer does not allow execCommand to run under design mode properly, we could
not test its behavior.

Tests: editing/editability/empty-document-delete.html

editing/editability/empty-document-justify-right.html
editing/editability/empty-document-stylewithcss.html
editing/execCommand/insert-image-with-selecting-document.html

  • editing/Editor.cpp: (WebCore::Editor::canEdit): Verify that the root editable element exists instead of just checking that selection endpoints are editable because selection endpoints could be document node without a body element in design mode and we don't want to consider such a document editable. (WebCore::Editor::canDelete): Ditto.
  • editing/EditorCommand.cpp: (WebCore::enabledInEditableText): Ditto. (WebCore::enabledInRichlyEditableText): Ditto. (WebCore::enabledDelete): Call enabledCut and enabledInEditableText instead of duplicating the code in order to fix the same bug.

2011-03-21 Ryosuke Niwa <​rniwa@webkit.org>

Reviewed by Eric Seidel.

editing commands shouldn't run when there's no body
​https://bugs.webkit.org/show_bug.cgi?id=56771

Added tests to ensure WebKit does not crash when attempted to execute editing commands
in an empty document. Also added a test to ensure WebKit does not crash when InsertImage
is executed with selection endpoints being document. WebKit should ignore such attempts
and should not crash.

  • editing/editability/empty-document-delete-expected.txt: Added.
  • editing/editability/empty-document-delete.html: Added.
  • editing/editability/empty-document-justify-right-expected.txt: Added.
  • editing/editability/empty-document-justify-right.html: Added.
  • editing/editability/empty-document-stylewithcss-expected.txt: Added.
  • editing/editability/empty-document-stylewithcss.html: Added.
  • editing/execCommand/insert-image-with-selecting-document-expected.txt: Added.
  • editing/execCommand/insert-image-with-selecting-document.html: Added.
Location:
trunk
Files:
8 added
4 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r82790 r82791  
     12011-03-21  Ryosuke Niwa  <rniwa@webkit.org>
     2
     3        Reviewed by Eric Seidel.
     4
     5        editing commands shouldn't run when there's no body
     6        https://bugs.webkit.org/show_bug.cgi?id=56771
     7
     8        Added tests to ensure WebKit does not crash when attempted to execute editing commands
     9        in an empty document. Also added a test to ensure WebKit does not crash when InsertImage
     10        is executed with selection endpoints being document. WebKit should ignore such attempts
     11        and should not crash.
     12
     13        * editing/editability/empty-document-delete-expected.txt: Added.
     14        * editing/editability/empty-document-delete.html: Added.
     15        * editing/editability/empty-document-justify-right-expected.txt: Added.
     16        * editing/editability/empty-document-justify-right.html: Added.
     17        * editing/editability/empty-document-stylewithcss-expected.txt: Added.
     18        * editing/editability/empty-document-stylewithcss.html: Added.
     19        * editing/execCommand/insert-image-with-selecting-document-expected.txt: Added.
     20        * editing/execCommand/insert-image-with-selecting-document.html: Added.
     21
    1222011-04-02  Dan Bernstein  <mitz@apple.com>
    223

  • trunk/Source/WebCore/ChangeLog

    r82788 r82791  
     12011-03-21  Ryosuke Niwa  <rniwa@webkit.org>
     2
     3        Reviewed by Eric Seidel.
     4
     5        editing commands shouldn't run when there's no body
     6        https://bugs.webkit.org/show_bug.cgi?id=56771
     7
     8        The bug was caused by WebKit's not checking the existence of root editable element
     9        in enabled* functions. Although isContentEditable returns true whenever we're in design mode,
     10        we should not run editing commands in a document without a body element editable because
     11        doing so results in appending a non-body element to the document node.
     12
     13        Fixed the bug by modifying various enabled* functions to ensure we have a root editable element.
     14        New behavior tries to match that of Firefox except StyleWithCSS, which Firefox seems to ignore
     15        when there are no body element. Since StyleWithCSS is a document's state or property, we allow
     16        execCommand('StyleWithCSS') even in a document without a body element.
     17
     18        WebKit's and Firefox's behaviors also deviate in insert-image-with-selecting-document.html.
     19        Whereas WebKit respects selection set by script and ignores execCommand, Firefox modifies
     20        the selection when document.write("x") is ran and successfully inserts image.
     21
     22        Thus, empty-document-delete.html and empty-document-justify-right.html both pass on Firefox
     23        while empty-document-stylewithcss.html and insert-image-with-selecting-document.html both fail.
     24
     25        Since Internet Explorer does not allow execCommand to run under design mode properly, we could
     26        not test its behavior.
     27
     28        Tests: editing/editability/empty-document-delete.html
     29               editing/editability/empty-document-justify-right.html
     30               editing/editability/empty-document-stylewithcss.html
     31               editing/execCommand/insert-image-with-selecting-document.html
     32
     33        * editing/Editor.cpp:
     34        (WebCore::Editor::canEdit): Verify that the root editable element exists
     35        instead of just checking that selection endpoints are editable because
     36        selection endpoints could be document node without a body element in design mode
     37        and we don't want to consider such a document editable.
     38        (WebCore::Editor::canDelete): Ditto.
     39        * editing/EditorCommand.cpp:
     40        (WebCore::enabledInEditableText): Ditto.
     41        (WebCore::enabledInRichlyEditableText): Ditto.
     42        (WebCore::enabledDelete): Call enabledCut and enabledInEditableText instead
     43        of duplicating the code in order to fix the same bug.
     44
    1452011-04-02  Dan Bernstein  <mitz@apple.com>
    246

  • trunk/Source/WebCore/editing/Editor.cpp

    r82533 r82791  
    211211bool Editor::canEdit() const
    212212{
    213     return m_frame->selection()->isContentEditable();
     213    return m_frame->selection()->rootEditableElement();
    214214}
    215215
    … …  
    279279{
    280280    SelectionController* selection = m_frame->selection();
    281     return selection->isRange() && selection->isContentEditable();
     281    return selection->isRange() && selection->rootEditableElement();
    282282}
    283283

  • trunk/Source/WebCore/editing/EditorCommand.cpp

    r82176 r82791  
    11971197}
    11981198
     1199static bool enabledInEditableText(Frame* frame, Event* event, EditorCommandSource)
     1200{
     1201    return frame->editor()->selectionForCommand(event).rootEditableElement();
     1202}
     1203
    11991204static bool enabledDelete(Frame* frame, Event* event, EditorCommandSource source)
    12001205{
    … …  
    12021207    case CommandFromMenuOrKeyBinding:
    12031208        // "Delete" from menu only affects selected range, just like Cut but without affecting pasteboard
    1204         return frame->editor()->canDHTMLCut() || frame->editor()->canCut();
     1209        return enabledCut(frame, event, source);
    12051210    case CommandFromDOM:
    12061211    case CommandFromDOMWithUserInterface:
    12071212        // "Delete" from DOM is like delete/backspace keypress, affects selected range if non-empty,
    12081213        // otherwise removes a character
    1209         return frame->editor()->selectionForCommand(event).isContentEditable();
     1214        return enabledInEditableText(frame, event, source);
    12101215    }
    12111216    ASSERT_NOT_REACHED();
    … …  
    12131218}
    12141219
    1215 static bool enabledInEditableText(Frame* frame, Event* event, EditorCommandSource)
    1216 {
    1217     return frame->editor()->selectionForCommand(event).isContentEditable();
    1218 }
    1219 
    12201220static bool enabledInEditableTextOrCaretBrowsing(Frame* frame, Event* event, EditorCommandSource)
    12211221{
    … …  
    12261226static bool enabledInRichlyEditableText(Frame* frame, Event*, EditorCommandSource)
    12271227{
    1228     return frame->selection()->isCaretOrRange() && frame->selection()->isContentRichlyEditable();
     1228    return frame->selection()->isCaretOrRange() && frame->selection()->isContentRichlyEditable() && frame->selection()->rootEditableElement();
    12291229}
    12301230
Note: See TracChangeset for help on using the changeset viewer.