Reviewed by Tim Hatcher.
Fixed <rdar://problem/
5420682> Mail crashes at WebCore::InsertLineBreakCommand::doApply() after dropping a selected image over container's close box
* editing/DeleteButtonController.cpp:
(WebCore::DeleteButtonController::show):
Factored out the code in ::show() that created and styled the elements of the Deletion UI
(WebCore::DeleteButtonController::createDeletionUI):
Neglecting to move the append of the deletionUI elements into the same clause that handles the creation
of them ended up creating multiple elements at were repeatedly appended to the target, resulting in a
bloated table deletion UI which was slow to show and hide.
* editing/DeleteButtonController.h:
(WebCore::DeleteButtonController::enabled):
Restore this function to how it used to be pre-r25305, sans asserts
* editing/EditCommand.cpp:
Add disable/enable sandwich when undoing/redoing commands too
(WebCore::EditCommand::unapply):
(WebCore::EditCommand::reapply):
LayoutTests:
Reviewed by Tim Hatcher.
Test for <rdar://problem/
5420682> Mail crashes at WebCore::InsertLineBreakCommand::doApply() after dropping a selected image over container's close box
* editing/deleting/deletionUI-single-instance.html: Added.
* platform/mac/editing/deleting/deletionUI-single-instance-expected.checksum: Added.
* platform/mac/editing/deleting/deletionUI-single-instance-expected.png: Added.
* platform/mac/editing/deleting/deletionUI-single-instance-expected.txt: Added.
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@25335
268f45cc-cd09-0410-ab3c-
d52691b4dbfc
+2007-08-31 Alice Liu <alice.liu@apple.com>
+
+ Reviewed by Tim Hatcher.
+
+ Test for <rdar://problem/5420682> Mail crashes at WebCore::InsertLineBreakCommand::doApply() after dropping a selected image over container's close box
+
+ * editing/deleting/deletionUI-single-instance.html: Added.
+ * platform/mac/editing/deleting/deletionUI-single-instance-expected.checksum: Added.
+ * platform/mac/editing/deleting/deletionUI-single-instance-expected.png: Added.
+ * platform/mac/editing/deleting/deletionUI-single-instance-expected.txt: Added.
+
2007-08-30 Adele Peterson <adele@apple.com>
Reviewed by Justin.
--- /dev/null
+<body>
+
+<p>
+This test can only be run with DRT and not manually from the browser. This test makes sure that we don't make the mistake of adding new m_containerElements to the DeletionUI over and over again.
+A successful run of the test will not have multiple instances of deletion UI render objects at the same coordinates:
+</p>
+
+<div contenteditable="true" style="padding: 1em;">
+<table class="needsDeletionUI">
+
+<tr>
+<td id="td">
+Lorem ipsum dolor sit amet, consectetuer adipiscing elit. Maecenas odio. Morbi sed tortor id nisl bibendum commodo. Donec pede. Praesent accumsan dui vitae mauris. Mauris non dui at neque lacinia pulvinar. Quisque nibh. Nulla vitae lectus. Pellentesque enim. Mauris hendrerit molestie dui. Etiam pretium ligula a pede. Fusce consectetuer purus sit amet sem. Morbi tincidunt mollis libero. Maecenas molestie.
+</td>
+</tr>
+</table>
+
+</div>
+
+<script>
+
+sel = window.getSelection();
+td = document.getElementById("td");
+sel.setPosition(td, td.textContent.length);
+
+if (window.layoutTestController) {
+ for (i = 0; i < 5; i++)
+ document.execCommand("Delete");
+}
+
+</script>
+</body>
--- /dev/null
+cc44dad406d4b46bbd4e1cfb20c2aba0
\ No newline at end of file
--- /dev/null
+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 784x54
+ RenderText {#text} at (0,0) size 782x54
+ text run at (0,0) width 457: "This test can only be run with DRT and not manually from the browser. "
+ text run at (457,0) width 325: "This test makes sure that we don't make the mistake"
+ text run at (0,18) width 738: "of adding new m_containerElements to the DeletionUI over and over again. A successful run of the test will not have"
+ text run at (0,36) width 450: "multiple instances of deletion UI render objects at the same coordinates:"
+ RenderBlock {DIV} at (0,70) size 784x110
+layer at (24,94) size 752x78 layerType: background only
+layer at (20,90) size 760x86
+ RenderBlock (positioned) zI: -1000000 {DIV} at (-4,-4) size 760x86 [border: (4px solid #00000099)]
+layer at (24,94) size 752x78 layerType: foreground only
+ RenderTable {TABLE} at (16,16) size 752x78
+ RenderTableSection {TBODY} at (0,0) size 752x78
+ RenderTableRow {TR} at (0,2) size 752x74
+ RenderTableCell {TD} at (2,2) size 748x74 [r=0 c=0 rs=1 cs=1]
+ RenderText {#text} at (1,1) size 746x72
+ text run at (1,1) width 743: "Lorem ipsum dolor sit amet, consectetuer adipiscing elit. Maecenas odio. Morbi sed tortor id nisl bibendum commodo."
+ text run at (744,1) width 3: " "
+ text run at (1,19) width 743: "Donec pede. Praesent accumsan dui vitae mauris. Mauris non dui at neque lacinia pulvinar. Quisque nibh. Nulla vitae "
+ text run at (1,37) width 719: "lectus. Pellentesque enim. Mauris hendrerit molestie dui. Etiam pretium ligula a pede. Fusce consectetuer purus sit "
+ text run at (1,55) width 353: "amet sem. Morbi tincidunt mollis libero. Maecenas mole"
+ RenderBlock {DIV} at (0,0) size 752x0
+layer at (7,79) size 30x30
+ RenderImage zI: 1000000 {IMG} at (-17,-15) size 30x30
+caret: position 403 of child 0 {#text} of child 1 {TD} of child 0 {TR} of child 1 {TBODY} of child 1 {TABLE} of child 3 {DIV} of child 0 {BODY} of child 0 {HTML} of document
+2007-08-31 Alice Liu <alice.liu@apple.com>
+
+ Reviewed by Tim Hatcher.
+
+ Fixed <rdar://problem/5420682> Mail crashes at WebCore::InsertLineBreakCommand::doApply() after dropping a selected image over container's close box
+
+ * editing/DeleteButtonController.cpp:
+ (WebCore::DeleteButtonController::show):
+ Factored out the code in ::show() that created and styled the elements of the Deletion UI
+
+ (WebCore::DeleteButtonController::createDeletionUI):
+ Neglecting to move the append of the deletionUI elements into the same clause that handles the creation
+ of them ended up creating multiple elements at were repeatedly appended to the target, resulting in a
+ bloated table deletion UI which was slow to show and hide.
+
+ * editing/DeleteButtonController.h:
+ (WebCore::DeleteButtonController::enabled):
+ Restore this function to how it used to be pre-r25305, sans asserts
+
+ * editing/EditCommand.cpp:
+ Add disable/enable sandwich when undoing/redoing commands too
+ (WebCore::EditCommand::unapply):
+ (WebCore::EditCommand::reapply):
+
2007-08-31 Antti Koivisto <antti@apple.com>
Reviewed by Anders.
hide();
}
-void DeleteButtonController::show(HTMLElement* element)
+void DeleteButtonController::createDeletionUI()
{
- hide();
-
- if (!enabled() || !element || !element->inDocument() || !isDeletableElement(element))
- return;
-
- if (!m_frame->editor()->shouldShowDeleteInterface(static_cast<HTMLElement*>(element)))
- return;
-
- // we rely on the renderer having current information, so we should update the layout if needed
- m_frame->document()->updateLayoutIgnorePendingStylesheets();
-
- m_target = element;
-
- if (!m_containerElement) {
- m_containerElement = new HTMLDivElement(m_target->document());
- m_containerElement->setId(containerElementIdentifier);
- }
+ RefPtr<HTMLDivElement> container = new HTMLDivElement(m_target->document());
+ container->setId(containerElementIdentifier);
- CSSMutableStyleDeclaration* style = m_containerElement->getInlineStyleDecl();
+ CSSMutableStyleDeclaration* style = container->getInlineStyleDecl();
style->setProperty(CSS_PROP__WEBKIT_USER_DRAG, CSS_VAL_NONE);
style->setProperty(CSS_PROP__WEBKIT_USER_SELECT, CSS_VAL_IGNORE);
style->setProperty(CSS_PROP__WEBKIT_USER_MODIFY, CSS_VAL_NONE);
- ExceptionCode ec = 0;
- m_target->appendChild(m_containerElement.get(), ec);
- ASSERT(ec == 0);
- if (ec) {
- hide();
- return;
- }
-
- m_outlineElement = new HTMLDivElement(m_target->document());
- m_outlineElement->setId(outlineElementIdentifier);
+ RefPtr<HTMLDivElement> outline = new HTMLDivElement(m_target->document());
+ outline->setId(outlineElementIdentifier);
const int borderWidth = 4;
const int borderRadius = 6;
- style = m_outlineElement->getInlineStyleDecl();
+ style = outline->getInlineStyleDecl();
style->setProperty(CSS_PROP_POSITION, CSS_VAL_ABSOLUTE);
style->setProperty(CSS_PROP_CURSOR, CSS_VAL_DEFAULT);
style->setProperty(CSS_PROP__WEBKIT_USER_DRAG, CSS_VAL_NONE);
style->setProperty(CSS_PROP_BORDER, String::number(borderWidth) + "px solid rgba(0, 0, 0, 0.6)");
style->setProperty(CSS_PROP__WEBKIT_BORDER_RADIUS, String::number(borderRadius) + "px");
- m_containerElement->appendChild(m_outlineElement.get(), ec);
+ ExceptionCode ec = 0;
+ container->appendChild(outline.get(), ec);
ASSERT(ec == 0);
- if (ec) {
- hide();
+ if (ec)
return;
- }
- m_buttonElement = new DeleteButton(m_target->document());
- m_buttonElement->setId(buttonElementIdentifier);
+ RefPtr<DeleteButton> button = new DeleteButton(m_target->document());
+ button->setId(buttonElementIdentifier);
const int buttonWidth = 30;
const int buttonHeight = 30;
const int buttonBottomShadowOffset = 2;
- style = m_buttonElement->getInlineStyleDecl();
+ style = button->getInlineStyleDecl();
style->setProperty(CSS_PROP_POSITION, CSS_VAL_ABSOLUTE);
style->setProperty(CSS_PROP_CURSOR, CSS_VAL_DEFAULT);
style->setProperty(CSS_PROP__WEBKIT_USER_DRAG, CSS_VAL_NONE);
style->setProperty(CSS_PROP_WIDTH, String::number(buttonWidth) + "px");
style->setProperty(CSS_PROP_HEIGHT, String::number(buttonHeight) + "px");
- m_buttonElement->setCachedImage(new CachedImage(Image::loadPlatformResource("deleteButton")));
+ button->setCachedImage(new CachedImage(Image::loadPlatformResource("deleteButton")));
- m_containerElement->appendChild(m_buttonElement.get(), ec);
+ container->appendChild(button.get(), ec);
+ ASSERT(ec == 0);
+ if (ec)
+ return;
+
+ m_containerElement = container.release();
+ m_outlineElement = outline.release();
+ m_buttonElement = button.release();
+}
+
+void DeleteButtonController::show(HTMLElement* element)
+{
+ hide();
+
+ if (!enabled() || !element || !element->inDocument() || !isDeletableElement(element))
+ return;
+
+ if (!m_frame->editor()->shouldShowDeleteInterface(static_cast<HTMLElement*>(element)))
+ return;
+
+ // we rely on the renderer having current information, so we should update the layout if needed
+ m_frame->document()->updateLayoutIgnorePendingStylesheets();
+
+ m_target = element;
+
+ if (!m_containerElement) {
+ createDeletionUI();
+ if (!m_containerElement) {
+ hide();
+ return;
+ }
+ }
+
+ ExceptionCode ec = 0;
+ m_target->appendChild(m_containerElement.get(), ec);
ASSERT(ec == 0);
if (ec) {
hide();
m_target->getInlineStyleDecl()->setProperty(CSS_PROP_Z_INDEX, "0");
m_wasAutoZIndex = true;
}
-
- m_target->renderer()->repaint(true);
}
void DeleteButtonController::hide()
void show(HTMLElement*);
void hide();
- bool enabled() const { return (m_disableStack == 0 || m_target); }
+ bool enabled() const { return (m_disableStack == 0); }
void enable();
void disable();
static const char* const buttonElementIdentifier;
static const char* const outlineElementIdentifier;
+ void createDeletionUI();
+
Frame* m_frame;
RefPtr<HTMLElement> m_target;
RefPtr<HTMLElement> m_containerElement;
Frame* frame = m_document->frame();
+ DeleteButtonController *deleteButtonController = frame->editor()->deleteButtonController();
+ deleteButtonController->disable();
doUnapply();
+ deleteButtonController->enable();
if (!m_parent) {
updateLayout();
Frame* frame = m_document->frame();
+ DeleteButtonController *deleteButtonController = frame->editor()->deleteButtonController();
+ deleteButtonController->disable();
doReapply();
+ deleteButtonController->enable();
if (!m_parent) {
updateLayout();