Copying, pasting, and then deleting an attachment element breaks attachment data...
authorwenson_hsieh@apple.com <wenson_hsieh@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 8 Jan 2018 22:19:15 +0000 (22:19 +0000)
committerwenson_hsieh@apple.com <wenson_hsieh@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 8 Jan 2018 22:19:15 +0000 (22:19 +0000)
commitd516083619ad255301eac09343559b5461895e98
treeb65ea8019994ed9eed6e11452b445e77557abf7e
parent20e556485b179866d5d0fc28d9acd9d61ef50647
Copying, pasting, and then deleting an attachment element breaks attachment data requests
https://bugs.webkit.org/show_bug.cgi?id=181365
<rdar://problem/36340647>

Reviewed by Tim Horton.

Source/WebCore:

Currently, copying and pasting an attachment element within the same document and then deleting backwards to
remove the pasted attachment element causes the original attachment element to be inaccessible via SPI. This is
because there are now two different attachment elements with the same unique identifier, such that Document,
which keeps a map of all unique attachment identifiers to attachment elements, will lose track of the original
attachment element.

To fix this, we ensure that attachment elements should always have unique identifiers when they are inserted
into the document. We make several small adjustments to accomplish this:

1.  First, refactor HTMLAttachmentElement's unique identifier so that it no longer depends on the value of the
    "webkitattachmentid" attribute, and is instead just a member of HTMLAttachmentElement that is not exposed to
    DOM bindings. This means setting and querying an attachment element's uniqueIdentifier can be done without
    triggering any side effects, such as layout or mutation events.

2.  Next, make "webkitattachmentid" a temporary attribute similar to "webkitattachmentpath" and
    "webkitattachmentbloburl", so that it is added only when generating a markup fragment for editing, and
    removed upon deserialization.

3.  Lastly, shift the responsibility of assigning a unique identifier to an attachment away from places where we
    create attachment elements, and instead have Document enforce this when an attachment element is inserted.

Tests:  WKAttachmentTests.InsertAndRemoveDuplicateAttachment
        WKAttachmentTests.InsertDuplicateAttachmentAndUpdateData

* dom/Document.cpp:
(WebCore::Document::didInsertAttachmentElement):

Assign the unique identifier of an attachment element that has been inserted. If the identifier already tracks
an existing attachment element in the document or is missing, reassign the identifier to a new value.

* editing/cocoa/WebContentReaderCocoa.mm:
(WebCore::createFragmentForImageAttachment):
(WebCore::replaceRichContentWithAttachments):
(WebCore::WebContentReader::readFilePaths):

Remove calls to setUniqueIdentifier here, since Document will assign a unique identifier upon insertion.

* editing/markup.cpp:
(WebCore::StyledMarkupAccumulator::appendCustomAttributes):
(WebCore::createFragmentFromMarkup):

Set the attachment's unique identifier to the value of the "webkitattachmentid" attribute. When moving existing
attachments around in the DOM without duplication, this ensures that the attachment will be removed and
reinserted in the document without triggering removal and insertion client delegate methods.

When pasting an attachment element that has the same identifier as an existing attachment, we let Document
realize that the attachment identifier already exists, and reassign it to a unique value.

* html/HTMLAttachmentElement.cpp:
(WebCore::HTMLAttachmentElement::uniqueIdentifier const): Deleted.
(WebCore::HTMLAttachmentElement::setUniqueIdentifier): Deleted.
* html/HTMLAttachmentElement.h:

Tools:

Adds two new attachment API tests to verify that copying and pasting an existing attachment inserts an
attachment element that may be edited independently of the original attachment. See WebCore/ChangeLog for more
detail.

* TestWebKitAPI/Tests/WebKitCocoa/WKAttachmentTests.mm:
(TestWebKitAPI::TEST):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@226539 268f45cc-cd09-0410-ab3c-d52691b4dbfc
Source/WebCore/ChangeLog
Source/WebCore/dom/Document.cpp
Source/WebCore/editing/cocoa/WebContentReaderCocoa.mm
Source/WebCore/editing/markup.cpp
Source/WebCore/html/HTMLAttachmentElement.cpp
Source/WebCore/html/HTMLAttachmentElement.h
Tools/ChangeLog
Tools/TestWebKitAPI/Tests/WebKitCocoa/WKAttachmentTests.mm