Bug 179016 – [Attachment Support] Implement delegate hooks for attachment element insertion and removal

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit Misc.

[Wenson Hsieh]

Clients need to know when attachment elements are either inserted into or removed from the document.

[Wenson Hsieh]

<rdar://problem/35250890>

[Wenson Hsieh]

Created attachment 326059 [details]
First pass

[Build Bot]

Comment on attachment 326059 [details]
First pass

Attachment 326059 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.webkit.org/results/5111786

New failing tests:
editing/pasteboard/copy-paste-attachment.html
fast/attachment/attachment-default-icon.html
fast/attachment/attachment-respects-css-size.html
fast/attachment/attachment-rendering.html
fast/attachment/attachment-subtitle.html
accessibility/attachment-element.html
fast/attachment/attachment-select-on-click-inside-user-select-all.html
editing/pasteboard/drag-and-drop-attachment-contenteditable.html
fast/attachment/attachment-select-on-click.html
fast/attachment/attachment-title.html
fast/attachment/attachment-icon-from-file-extension.html
accessibility/mac/attachment-element-replacement-character.html
fast/attachment/attachment-subtitle-resize.html
fast/attachment/attachment-progress.html
fast/attachment/attachment-label-highlight.html
fast/attachment/attachment-uti.html
editing/pasteboard/drag-files-to-editable-element-as-attachment.html
fast/attachment/attachment-action.html
fast/attachment/attachment-folder-icon.html
fast/attachment/attachment-type-attribute.html

[Build Bot]

Created attachment 326061 [details]
Archive of layout-test-results from ews102 for mac-elcapitan

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews102  Port: mac-elcapitan  Platform: Mac OS X 10.11.6

[Build Bot]

Comment on attachment 326059 [details]
First pass

Attachment 326059 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: http://webkit-queues.webkit.org/results/5111782

New failing tests:
editing/pasteboard/copy-paste-attachment.html
fast/attachment/attachment-default-icon.html
fast/attachment/attachment-respects-css-size.html
fast/attachment/attachment-rendering.html
fast/attachment/attachment-wrapping-action.html
accessibility/attachment-element.html
fast/attachment/attachment-select-on-click-inside-user-select-all.html
fast/attachment/attachment-select-on-click.html
accessibility/ios-simulator/attributed-string-for-range.html
fast/attachment/attachment-icon-from-file-extension.html
fast/attachment/attachment-action.html
fast/attachment/attachment-progress.html
fast/attachment/attachment-borderless.html
fast/attachment/attachment-label-highlight.html
fast/attachment/attachment-title.html
fast/attachment/attachment-subtitle.html
fast/attachment/attachment-uti.html

[Build Bot]

Created attachment 326062 [details]
Archive of layout-test-results from ews124 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews124  Port: ios-simulator-wk2  Platform: Mac OS X 10.12.6

[Build Bot]

Comment on attachment 326059 [details]
First pass

Attachment 326059 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.webkit.org/results/5111811

New failing tests:
editing/pasteboard/copy-paste-attachment.html
fast/attachment/attachment-default-icon.html
fast/attachment/attachment-respects-css-size.html
fast/attachment/attachment-rendering.html
fast/attachment/attachment-subtitle.html
accessibility/attachment-element.html
fast/attachment/attachment-select-on-click-inside-user-select-all.html
fast/attachment/attachment-select-on-click.html
fast/attachment/attachment-icon-from-file-extension.html
accessibility/mac/attachment-element-replacement-character.html
fast/attachment/attachment-subtitle-resize.html
fast/attachment/attachment-progress.html
fast/attachment/attachment-uti.html
fast/attachment/attachment-label-highlight.html
fast/attachment/attachment-action.html
fast/attachment/attachment-title.html
fast/attachment/attachment-type-attribute.html
fast/attachment/attachment-folder-icon.html

[Build Bot]

Created attachment 326063 [details]
Archive of layout-test-results from ews106 for mac-elcapitan-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews106  Port: mac-elcapitan-wk2  Platform: Mac OS X 10.11.6

[Build Bot]

Comment on attachment 326059 [details]
First pass

Attachment 326059 [details] did not pass mac-debug-ews (mac):
Output: http://webkit-queues.webkit.org/results/5111797

New failing tests:
editing/pasteboard/copy-paste-attachment.html
fast/attachment/attachment-default-icon.html
fast/attachment/attachment-respects-css-size.html
fast/attachment/attachment-rendering.html
fast/attachment/attachment-subtitle.html
accessibility/attachment-element.html
fast/attachment/attachment-select-on-click-inside-user-select-all.html
fast/attachment/attachment-folder-icon.html
fast/attachment/attachment-select-on-click.html
fast/attachment/attachment-icon-from-file-extension.html
fast/attachment/attachment-subtitle-resize.html
accessibility/mac/attachment-element-replacement-character.html
fast/attachment/attachment-type-attribute.html
editing/pasteboard/drag-and-drop-attachment-contenteditable.html
fast/attachment/attachment-progress.html
fast/attachment/attachment-label-highlight.html
fast/attachment/attachment-action.html
fast/attachment/attachment-title.html
editing/pasteboard/drag-files-to-editable-element-as-attachment.html
fast/attachment/attachment-uti.html

[Build Bot]

Created attachment 326064 [details]
Archive of layout-test-results from ews114 for mac-elcapitan

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews114  Port: mac-elcapitan  Platform: Mac OS X 10.11.6

[Wenson Hsieh]

Created attachment 326070 [details]
Patch

[Wenson Hsieh]

Comment on attachment 326070 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=326070&action=review

Thanks Tim! (I found a couple of other little things to fix, so I'll make some tweaks before landing).

> Source/WebCore/html/HTMLAttachmentElement.h:70
>      void parseAttribute(const QualifiedName&, const AtomicString&) final;

I also meant to remove the declaration of the `HTMLAttachmentElement(const QualifiedName&, Document&, const String& identifier);` constructor above — I'll do this before landing.

> Source/WebKit/UIProcess/Cocoa/PageClientImplCocoa.mm:53
> +#endif

Self nit - this should have an #else UNUSED_PARAM(identifier), otherwise the build would break with an unused variable if we were to somehow have ENABLE_ATTACHMENT_ELEMENT without WK_API_ENABLED.

> Source/WebKit/UIProcess/Cocoa/PageClientImplCocoa.mm:60
> +#endif

Ditto.

[Wenson Hsieh]

Created attachment 326151 [details]
Patch for landing

[WebKit Commit Bot]

Comment on attachment 326151 [details]
Patch for landing

Clearing flags on attachment: 326151

Committed r224512: <https://trac.webkit.org/changeset/224512>

[Ryan Haddad]

WKAttachmentTests.AttachmentUpdatesWhenChangingFontStyles is failing on debug bots:

ASSERTION FAILED: !textDecorationsInEffect || !textDecoration
/Volumes/Data/slave/highsierra-debug/build/Source/WebCore/editing/EditingStyle.cpp(1544) : void WebCore::reconcileTextDecorationProperties(WebCore::MutableStyleProperties *)
1   0x49ef288fd WTFCrash
2   0x491e3f694 WebCore::reconcileTextDecorationProperties(WebCore::MutableStyleProperties*)
3   0x491e3f004 WebCore::StyleChange::StyleChange(WebCore::EditingStyle*, WebCore::Position const&)
4   0x491e3fce5 WebCore::StyleChange::StyleChange(WebCore::EditingStyle*, WebCore::Position const&)
5   0x491e0bf94 WebCore::ApplyStyleCommand::applyInlineStyleToNodeRange(WebCore::EditingStyle&, WebCore::Node&, WebCore::Node*)
6   0x491e0b76d WebCore::ApplyStyleCommand::fixRangeAndApplyInlineStyle(WebCore::EditingStyle&, WebCore::Position const&, WebCore::Position const&)
7   0x491e0789c WebCore::ApplyStyleCommand::applyInlineStyle(WebCore::EditingStyle&)
8   0x491e05439 WebCore::ApplyStyleCommand::doApply()
9   0x491e12eef WebCore::CompositeEditCommand::applyCommandToComposite(WTF::Ref<WebCore::EditCommand>&&)
10  0x491e13423 WebCore::CompositeEditCommand::applyStyle(WebCore::EditingStyle const*, WebCore::EditAction)
11  0x491e86867 WebCore::InsertTextCommand::doApply()
12  0x491e13242 WebCore::CompositeEditCommand::applyCommandToComposite(WTF::Ref<WebCore::CompositeEditCommand>&&, WebCore::VisibleSelection const&)
13  0x491eb5d68 WebCore::TypingCommand::insertTextRunWithoutNewlines(WTF::String const&, bool)
14  0x491ecfafc WebCore::TypingCommandLineOperation::operator()(unsigned long, unsigned long, bool) const
15  0x491eb5c19 void WebCore::forEachLineInString<WebCore::TypingCommandLineOperation>(WTF::String const&, WebCore::TypingCommandLineOperation const&)
16  0x491eb5b38 WebCore::TypingCommand::insertText(WTF::String const&, bool)
17  0x491eb41d5 WebCore::TypingCommand::insertTextAndNotifyAccessibility(WTF::String const&, bool)
18  0x491eb4e10 WebCore::TypingCommand::doApply()
19  0x491dff08a WebCore::CompositeEditCommand::apply()
20  0x491ea4ff3 WebCore::TextInsertionBaseCommand::applyTextInsertionCommand(WebCore::Frame*, WebCore::TextInsertionBaseCommand&, WebCore::VisibleSelection const&, WebCore::VisibleSelection const&)
21  0x491eb408c WebCore::TypingCommand::insertText(WebCore::Document&, WTF::String const&, WebCore::VisibleSelection const&, unsigned int, WebCore::TypingCommand::TextCompositionType)
22  0x491eb3d6b WebCore::TypingCommand::insertText(WebCore::Document&, WTF::String const&, unsigned int, WebCore::TypingCommand::TextCompositionType)
23  0x491e5e6f4 WebCore::executeInsertText(WebCore::Frame&, WebCore::Event*, WebCore::EditorCommandSource, WTF::String const&)
24  0x491e4b363 WebCore::Editor::Command::execute(WTF::String const&, WebCore::Event*) const
25  0x1043d9583 WebKit::WebPage::executeEditingCommand(WTF::String const&, WTF::String const&)
26  0x1043d8d45 WebKit::WebPage::executeEditCommand(WTF::String const&, WTF::String const&)
27  0x1043d8ceb WebKit::WebPage::executeEditCommandWithCallback(WTF::String const&, WTF::String const&, WebKit::CallbackID)
28  0x10446b1c8 void IPC::callMemberFunctionImpl<WebKit::WebPage, void (WebKit::WebPage::*)(WTF::String const&, WTF::String const&, WebKit::CallbackID), std::__1::tuple<WTF::String, WTF::String, WebKit::CallbackID>, 0ul, 1ul, 2ul>(WebKit::WebPage*, void (WebKit::WebPage::*)(WTF::String const&, WTF::String const&, WebKit::CallbackID), std::__1::tuple<WTF::String, WTF::String, WebKit::CallbackID>&&, std::__1::integer_sequence<unsigned long, 0ul, 1ul, 2ul>)
29  0x10446ade0 void IPC::callMemberFunction<WebKit::WebPage, void (WebKit::WebPage::*)(WTF::String const&, WTF::String const&, WebKit::CallbackID), std::__1::tuple<WTF::String, WTF::String, WebKit::CallbackID>, std::__1::integer_sequence<unsigned long, 0ul, 1ul, 2ul> >(std::__1::tuple<WTF::String, WTF::String, WebKit::CallbackID>&&, WebKit::WebPage*, void (WebKit::WebPage::*)(WTF::String const&, WTF::String const&, WebKit::CallbackID))
30  0x10445a577 void IPC::handleMessage<Messages::WebPage::ExecuteEditCommandWithCallback, WebKit::WebPage, void (WebKit::WebPage::*)(WTF::String const&, WTF::String const&, WebKit::CallbackID)>(IPC::Decoder&, WebKit::WebPage*, void (WebKit::WebPage::*)(WTF::String const&, WTF::String const&, WebKit::CallbackID))
31  0x1044539da WebKit::WebPage::didReceiveWebPageMessage(IPC::Connection&, IPC::Decoder&)
2017-11-07 03:38:07.351 TestWebKitAPI[63790:10229547] Expected removed attachments: (
) to match (
    "<_WKAttachment id='45415eb0-d28a-4cb5-8686-ef536d90f4ed'>"
).
FAIL WKAttachmentTests.AttachmentUpdatesWhenChangingFontStyles

/Volumes/Data/slave/highsierra-debug/build/Tools/TestWebKitAPI/Tests/WebKitCocoa/WKAttachmentTests.mm:174
Value of: [self _synchronouslyExecuteEditCommand:command argument:argument]
  Actual: false
Expected: true


/Volumes/Data/slave/highsierra-debug/build/Tools/TestWebKitAPI/Tests/WebKitCocoa/WKAttachmentTests.mm:101
Value of: removedAttachmentsMatch
  Actual: false
Expected: true

https://build.webkit.org/builders/Apple%20High%20Sierra%20Debug%20WK2%20(Tests)/builds/712

[Ryan Haddad]

Rolled out in <https://trac.webkit.org/changeset/224544>

[Wenson Hsieh]

Upon further investigation, this API test runs into an existing bug related to text decorations — this is reproducible with the following simple layout test (run from within LayoutTests/editing/execCommand/)

<body contenteditable></body>
<script>
document.body.focus();
document.execCommand("InsertHTML", true, "<img src='../resources/abe.png'></img>");
document.execCommand("SelectAll");
document.execCommand("Underline");
document.execCommand("InsertText", true, "foo");
</script>

[Wenson Hsieh]

Re-landed with a tweaked API test in <https://trac.webkit.org/r224593>. I've investigated the debug assertion, and determined that it's a separate (and existing) issue that has nothing to do with attachments. See <https://bugs.webkit.org/show_bug.cgi?id=179431> for additional analysis, as well as several proposals for a fix.