Changeset 224593 in webkit


Ignore:
Timestamp:
Nov 8, 2017, 1:10:00 PM (8 years ago)
Author:
Wenson Hsieh
Message:

[Attachment Support] Implement delegate hooks for attachment element insertion and removal
​https://bugs.webkit.org/show_bug.cgi?id=179016
<rdar://problem/35250890>

Reviewed by Tim Horton.

Source/WebCore:

Relanding this patch with a tweaked API test. WKAttachmentTests.AttachmentUpdatesWhenChangingFontStyles, in its
original form, hit a debug assertion and exposed an existing bug related to EditingStyles and text decoration.
After some investigation, this debug assertion is unrelated to the attachment logic introduced here; see
<​https://bugs.webkit.org/show_bug.cgi?id=179431> for additional analysis, and several proposed fixes.

  • editing/Editor.cpp:

(WebCore::Editor::respondToChangedSelection):
(WebCore::Editor::editorUIUpdateTimerFired):
(WebCore::Editor::scheduleEditorUIUpdate):
(WebCore::Editor::didInsertAttachmentElement):
(WebCore::Editor::didRemoveAttachmentElement):
(WebCore::Editor::notifyClientOfAttachmentUpdates):
(WebCore::Editor::insertAttachmentFromFile):

  • editing/Editor.h:
  • html/HTMLAttachmentElement.cpp:

(WebCore::HTMLAttachmentElement::HTMLAttachmentElement):
(WebCore::HTMLAttachmentElement::insertedIntoAncestor):
(WebCore::HTMLAttachmentElement::removedFromAncestor):
(WebCore::HTMLAttachmentElement::uniqueIdentifier const):
(WebCore::HTMLAttachmentElement::setUniqueIdentifier):

  • html/HTMLAttachmentElement.h:
  • html/HTMLAttributeNames.in:
  • page/EditorClient.h:

(WebCore::EditorClient::didInsertAttachment):
(WebCore::EditorClient::didRemoveAttachment):

Source/WebKit:

See WebCore/ChangeLog for more details.

  • UIProcess/API/Cocoa/WKUIDelegatePrivate.h:
  • UIProcess/API/Cocoa/WKWebView.mm:

(-[WKWebView _didInsertAttachment:]):
(-[WKWebView _didRemoveAttachment:]):

  • UIProcess/API/Cocoa/WKWebViewInternal.h:
  • UIProcess/Cocoa/PageClientImplCocoa.h:
  • UIProcess/Cocoa/PageClientImplCocoa.mm:

(WebKit::PageClientImplCocoa::didInsertAttachment):
(WebKit::PageClientImplCocoa::didRemoveAttachment):

  • UIProcess/PageClient.h:

(WebKit::PageClient::didInsertAttachment):
(WebKit::PageClient::didRemoveAttachment):

  • UIProcess/WebPageProxy.cpp:

(WebKit::WebPageProxy::didInsertAttachment):
(WebKit::WebPageProxy::didRemoveAttachment):

  • UIProcess/WebPageProxy.h:
  • UIProcess/WebPageProxy.messages.in:
  • WebProcess/WebCoreSupport/WebEditorClient.cpp:

(WebKit::WebEditorClient::didInsertAttachment):
(WebKit::WebEditorClient::didRemoveAttachment):

  • WebProcess/WebCoreSupport/WebEditorClient.h:

Tools:

See WebCore/ChangeLog for more details.

  • TestWebKitAPI/Tests/WebKitCocoa/WKAttachmentTests.mm:

(-[AttachmentUpdateObserver init]):
(-[AttachmentUpdateObserver inserted]):
(-[AttachmentUpdateObserver removed]):
(-[AttachmentUpdateObserver _webView:didInsertAttachment:]):
(-[AttachmentUpdateObserver _webView:didRemoveAttachment:]):
(TestWebKitAPI::ObserveAttachmentUpdatesForScope::ObserveAttachmentUpdatesForScope):
(TestWebKitAPI::ObserveAttachmentUpdatesForScope::~ObserveAttachmentUpdatesForScope):
(TestWebKitAPI::ObserveAttachmentUpdatesForScope::observer const):
(TestWebKitAPI::ObserveAttachmentUpdatesForScope::expectAttachmentUpdates):
(-[TestWKWebView _synchronouslyExecuteEditCommand:argument:]):
(-[TestWKWebView expectUpdatesAfterCommand:withArgument:expectedRemovals:expectedInsertions:]):
(TestWebKitAPI::TEST):

Location:
trunk
Files:
21 edited

Legend:

Unmodified
Added
Removed

  • TabularUnified trunk/Source/WebCore/ChangeLog

    r224591 r224593  
     12017-11-08  Wenson Hsieh  <wenson_hsieh@apple.com>
     2
     3        [Attachment Support] Implement delegate hooks for attachment element insertion and removal
     4        https://bugs.webkit.org/show_bug.cgi?id=179016
     5        <rdar://problem/35250890>
     6
     7        Reviewed by Tim Horton.
     8
     9        Relanding this patch with a tweaked API test. WKAttachmentTests.AttachmentUpdatesWhenChangingFontStyles, in its
     10        original form, hit a debug assertion and exposed an existing bug related to EditingStyles and text decoration.
     11        After some investigation, this debug assertion is unrelated to the attachment logic introduced here; see
     12        <https://bugs.webkit.org/show_bug.cgi?id=179431> for additional analysis, and several proposed fixes.
     13
     14        * editing/Editor.cpp:
     15        (WebCore::Editor::respondToChangedSelection):
     16        (WebCore::Editor::editorUIUpdateTimerFired):
     17        (WebCore::Editor::scheduleEditorUIUpdate):
     18        (WebCore::Editor::didInsertAttachmentElement):
     19        (WebCore::Editor::didRemoveAttachmentElement):
     20        (WebCore::Editor::notifyClientOfAttachmentUpdates):
     21        (WebCore::Editor::insertAttachmentFromFile):
     22        * editing/Editor.h:
     23        * html/HTMLAttachmentElement.cpp:
     24        (WebCore::HTMLAttachmentElement::HTMLAttachmentElement):
     25        (WebCore::HTMLAttachmentElement::insertedIntoAncestor):
     26        (WebCore::HTMLAttachmentElement::removedFromAncestor):
     27        (WebCore::HTMLAttachmentElement::uniqueIdentifier const):
     28        (WebCore::HTMLAttachmentElement::setUniqueIdentifier):
     29        * html/HTMLAttachmentElement.h:
     30        * html/HTMLAttributeNames.in:
     31        * page/EditorClient.h:
     32        (WebCore::EditorClient::didInsertAttachment):
     33        (WebCore::EditorClient::didRemoveAttachment):
     34
    1352017-11-08  Chris Dumez  <cdumez@apple.com>
    236

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

    r224544 r224593  
    34293429        && !(options & FrameSelection::SpellCorrectionTriggered);
    34303430    m_editorUIUpdateTimerWasTriggeredByDictation = options & FrameSelection::DictationTriggered;
    3431     m_editorUIUpdateTimer.startOneShot(0_s);
     3431    scheduleEditorUIUpdate();
    34323432}
    34333433
    … …  
    36023602
    36033603    m_oldSelectionForEditorUIUpdate = m_frame.selection().selection();
     3604
     3605#if ENABLE(ATTACHMENT_ELEMENT)
     3606    notifyClientOfAttachmentUpdates();
     3607#endif
    36043608}
    36053609
    … …  
    37343738}
    37353739
     3740void Editor::scheduleEditorUIUpdate()
     3741{
     3742    m_editorUIUpdateTimer.startOneShot(0_s);
     3743}
     3744
    37363745#if ENABLE(ATTACHMENT_ELEMENT)
     3746
     3747void Editor::didInsertAttachmentElement(HTMLAttachmentElement& attachment)
     3748{
     3749    auto identifier = attachment.uniqueIdentifier();
     3750    if (identifier.isEmpty())
     3751        return;
     3752
     3753    if (!m_removedAttachmentIdentifiers.take(identifier))
     3754        m_insertedAttachmentIdentifiers.add(identifier);
     3755    scheduleEditorUIUpdate();
     3756}
     3757
     3758void Editor::didRemoveAttachmentElement(HTMLAttachmentElement& attachment)
     3759{
     3760    auto identifier = attachment.uniqueIdentifier();
     3761    if (identifier.isEmpty())
     3762        return;
     3763
     3764    if (!m_insertedAttachmentIdentifiers.take(identifier))
     3765        m_removedAttachmentIdentifiers.add(identifier);
     3766    scheduleEditorUIUpdate();
     3767}
     3768
     3769void Editor::notifyClientOfAttachmentUpdates()
     3770{
     3771    if (auto* editorClient = client()) {
     3772        for (auto& identifier : m_removedAttachmentIdentifiers)
     3773            editorClient->didRemoveAttachment(identifier);
     3774        for (auto& identifier : m_insertedAttachmentIdentifiers)
     3775            editorClient->didInsertAttachment(identifier);
     3776    }
     3777
     3778    m_removedAttachmentIdentifiers.clear();
     3779    m_insertedAttachmentIdentifiers.clear();
     3780}
    37373781
    37383782void Editor::insertAttachment(const String& identifier, const String& filename, const String& filepath, std::optional<String> contentType)
    … …  
    37523796void Editor::insertAttachmentFromFile(const String& identifier, const String& filename, const String& contentType, Ref<File>&& file)
    37533797{
    3754     auto attachment = HTMLAttachmentElement::create(HTMLNames::attachmentTag, document(), identifier);
     3798    auto attachment = HTMLAttachmentElement::create(HTMLNames::attachmentTag, document());
    37553799    attachment->setAttribute(HTMLNames::titleAttr, filename);
    37563800    attachment->setAttribute(HTMLNames::subtitleAttr, fileSizeDescription(file->size()));

  • TabularUnified trunk/Source/WebCore/editing/Editor.h

    r224544 r224593  
    504504    WEBCORE_EXPORT void insertAttachment(const String& identifier, const String& filename, const String& filepath, std::optional<String> contentType = std::nullopt);
    505505    WEBCORE_EXPORT void insertAttachment(const String& identifier, const String& filename, Ref<SharedBuffer>&& data, std::optional<String> contentType = std::nullopt);
     506    void didInsertAttachmentElement(HTMLAttachmentElement&);
     507    void didRemoveAttachmentElement(HTMLAttachmentElement&);
    506508#endif
    507509
    … …  
    549551    static RefPtr<SharedBuffer> dataInRTFDFormat(NSAttributedString *);
    550552    static RefPtr<SharedBuffer> dataInRTFFormat(NSAttributedString *);
     553#endif
     554
     555    void scheduleEditorUIUpdate();
     556
     557#if ENABLE(ATTACHMENT_ELEMENT)
     558    void notifyClientOfAttachmentUpdates();
    551559#endif
    552560
    … …  
    570578    bool m_overwriteModeEnabled { false };
    571579
     580#if ENABLE(ATTACHMENT_ELEMENT)
     581    HashSet<String> m_insertedAttachmentIdentifiers;
     582    HashSet<String> m_removedAttachmentIdentifiers;
     583#endif
     584
    572585    VisibleSelection m_oldSelectionForEditorUIUpdate;
    573586    Timer m_editorUIUpdateTimer;

  • TabularUnified trunk/Source/WebCore/html/HTMLAttachmentElement.cpp

    r224544 r224593  
    3434#include "HTMLNames.h"
    3535#include "RenderAttachment.h"
    36 #include <wtf/UUID.h>
    3736
    3837namespace WebCore {
    … …  
    4039using namespace HTMLNames;
    4140
    42 HTMLAttachmentElement::HTMLAttachmentElement(const QualifiedName& tagName, Document& document, const String& identifier)
     41HTMLAttachmentElement::HTMLAttachmentElement(const QualifiedName& tagName, Document& document)
    4342    : HTMLElement(tagName, document)
    44     , m_uniqueIdentifier(identifier)
    4543{
    4644    ASSERT(hasTagName(attachmentTag));
    47 }
    48 
    49 HTMLAttachmentElement::HTMLAttachmentElement(const QualifiedName& tagName, Document& document)
    50     : HTMLAttachmentElement(tagName, document, createCanonicalUUIDString())
    51 {
    5245}
    5346
    … …  
    5750{
    5851    return adoptRef(*new HTMLAttachmentElement(tagName, document));
    59 }
    60 
    61 Ref<HTMLAttachmentElement> HTMLAttachmentElement::create(const QualifiedName& tagName, Document& document, const String& identifier)
    62 {
    63     return adoptRef(*new HTMLAttachmentElement(tagName, document, identifier));
    6452}
    6553
    … …  
    8270}
    8371
     72Node::InsertedIntoAncestorResult HTMLAttachmentElement::insertedIntoAncestor(InsertionType type, ContainerNode& ancestor)
     73{
     74    auto result = HTMLElement::insertedIntoAncestor(type, ancestor);
     75    if (auto* frame = document().frame()) {
     76        if (type.connectedToDocument)
     77            frame->editor().didInsertAttachmentElement(*this);
     78    }
     79    return result;
     80}
     81
     82void HTMLAttachmentElement::removedFromAncestor(RemovalType type, ContainerNode& ancestor)
     83{
     84    HTMLElement::removedFromAncestor(type, ancestor);
     85    if (auto* frame = document().frame()) {
     86        if (type.disconnectedFromDocument)
     87            frame->editor().didRemoveAttachmentElement(*this);
     88    }
     89}
     90
    8491void HTMLAttachmentElement::parseAttribute(const QualifiedName& name, const AtomicString& value)
    8592{
    … …  
    9097
    9198    HTMLElement::parseAttribute(name, value);
     99}
     100
     101String HTMLAttachmentElement::uniqueIdentifier() const
     102{
     103    return attributeWithoutSynchronization(HTMLNames::webkitattachmentidAttr);
     104}
     105
     106void HTMLAttachmentElement::setUniqueIdentifier(const String& identifier)
     107{
     108    setAttributeWithoutSynchronization(HTMLNames::webkitattachmentidAttr, identifier);
    92109}
    93110

  • TabularUnified trunk/Source/WebCore/html/HTMLAttachmentElement.h

    r224544 r224593  
    3838public:
    3939    static Ref<HTMLAttachmentElement> create(const QualifiedName&, Document&);
    40     static Ref<HTMLAttachmentElement> create(const QualifiedName&, Document&, const String& identifier);
    4140
    4241    WEBCORE_EXPORT File* file() const;
    4342    void setFile(File*);
    4443
    45     WEBCORE_EXPORT String uniqueIdentifier() const { return m_uniqueIdentifier; }
    46     void setUniqueIdentifier(const String& identifier) { m_uniqueIdentifier = identifier; }
     44    WEBCORE_EXPORT String uniqueIdentifier() const;
     45    void setUniqueIdentifier(const String&);
     46
     47    InsertedIntoAncestorResult insertedIntoAncestor(InsertionType, ContainerNode&) final;
     48    void removedFromAncestor(RemovalType, ContainerNode&) final;
    4749
    4850    WEBCORE_EXPORT String attachmentTitle() const;
    … …  
    5355private:
    5456    HTMLAttachmentElement(const QualifiedName&, Document&);
    55     HTMLAttachmentElement(const QualifiedName&, Document&, const String& identifier);
    5657    virtual ~HTMLAttachmentElement();
    5758
    … …  
    6970   
    7071    RefPtr<File> m_file;
    71     String m_uniqueIdentifier;
    7272};
    7373

  • TabularUnified trunk/Source/WebCore/html/HTMLAttributeNames.in

    r224544 r224593  
    382382vspace
    383383webkitallowfullscreen
     384webkitattachmentid
    384385webkitattachmentpath
    385386webkitdirectory

  • TabularUnified trunk/Source/WebCore/page/EditorClient.h

    r224544 r224593  
    7272    virtual void didApplyStyle() = 0;
    7373    virtual bool shouldMoveRangeAfterDelete(Range*, Range*) = 0;
     74
     75#if ENABLE(ATTACHMENT_ELEMENT)
     76    virtual void didInsertAttachment(const String&) { }
     77    virtual void didRemoveAttachment(const String&) { }
     78#endif
    7479
    7580    virtual void didBeginEditing() = 0;

  • TabularUnified trunk/Source/WebKit/ChangeLog

    r224575 r224593  
     12017-11-08  Wenson Hsieh  <wenson_hsieh@apple.com>
     2
     3        [Attachment Support] Implement delegate hooks for attachment element insertion and removal
     4        https://bugs.webkit.org/show_bug.cgi?id=179016
     5        <rdar://problem/35250890>
     6
     7        Reviewed by Tim Horton.
     8
     9        See WebCore/ChangeLog for more details.
     10
     11        * UIProcess/API/Cocoa/WKUIDelegatePrivate.h:
     12        * UIProcess/API/Cocoa/WKWebView.mm:
     13        (-[WKWebView _didInsertAttachment:]):
     14        (-[WKWebView _didRemoveAttachment:]):
     15        * UIProcess/API/Cocoa/WKWebViewInternal.h:
     16        * UIProcess/Cocoa/PageClientImplCocoa.h:
     17        * UIProcess/Cocoa/PageClientImplCocoa.mm:
     18        (WebKit::PageClientImplCocoa::didInsertAttachment):
     19        (WebKit::PageClientImplCocoa::didRemoveAttachment):
     20        * UIProcess/PageClient.h:
     21        (WebKit::PageClient::didInsertAttachment):
     22        (WebKit::PageClient::didRemoveAttachment):
     23        * UIProcess/WebPageProxy.cpp:
     24        (WebKit::WebPageProxy::didInsertAttachment):
     25        (WebKit::WebPageProxy::didRemoveAttachment):
     26        * UIProcess/WebPageProxy.h:
     27        * UIProcess/WebPageProxy.messages.in:
     28        * WebProcess/WebCoreSupport/WebEditorClient.cpp:
     29        (WebKit::WebEditorClient::didInsertAttachment):
     30        (WebKit::WebEditorClient::didRemoveAttachment):
     31        * WebProcess/WebCoreSupport/WebEditorClient.h:
     32
    1332017-11-08  Carlos Garcia Campos  <cgarcia@igalia.com>
    234

  • TabularUnified trunk/Source/WebKit/UIProcess/API/Cocoa/WKUIDelegatePrivate.h

    r224544 r224593  
    111111- (void)_webView:(WKWebView *)webView editorStateDidChange:(NSDictionary *)editorState WK_API_AVAILABLE(macosx(WK_MAC_TBA), ios(WK_IOS_TBA));
    112112
     113- (void)_webView:(WKWebView *)webView didRemoveAttachment:(_WKAttachment *)attachment WK_API_AVAILABLE(macosx(WK_MAC_TBA), ios(WK_IOS_TBA));
     114- (void)_webView:(WKWebView *)webView didInsertAttachment:(_WKAttachment *)attachment WK_API_AVAILABLE(macosx(WK_MAC_TBA), ios(WK_IOS_TBA));
     115
    113116#if TARGET_OS_IPHONE
    114117- (BOOL)_webView:(WKWebView *)webView shouldIncludeAppLinkActionsForElement:(_WKActivatedElementInfo *)element WK_API_AVAILABLE(ios(9.0));

  • TabularUnified trunk/Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm

    r224558 r224593  
    11971197        [uiDelegate _webView:self editorStateDidChange:dictionaryRepresentationForEditorState(_page->editorState())];
    11981198}
     1199
     1200#if ENABLE(ATTACHMENT_ELEMENT)
     1201
     1202- (void)_didInsertAttachment:(NSString *)identifier
     1203{
     1204    id <WKUIDelegatePrivate> uiDelegate = (id <WKUIDelegatePrivate>)self.UIDelegate;
     1205    if ([uiDelegate respondsToSelector:@selector(_webView:didInsertAttachment:)])
     1206        [uiDelegate _webView:self didInsertAttachment:[wrapper(API::Attachment::create(identifier, *_page).leakRef()) autorelease]];
     1207}
     1208
     1209- (void)_didRemoveAttachment:(NSString *)identifier
     1210{
     1211    id <WKUIDelegatePrivate> uiDelegate = (id <WKUIDelegatePrivate>)self.UIDelegate;
     1212    if ([uiDelegate respondsToSelector:@selector(_webView:didRemoveAttachment:)])
     1213        [uiDelegate _webView:self didRemoveAttachment:[wrapper(API::Attachment::create(identifier, *_page).leakRef()) autorelease]];
     1214}
     1215
     1216#endif // ENABLE(ATTACHMENT_ELEMENT)
    11991217
    12001218#pragma mark iOS-specific methods

  • TabularUnified trunk/Source/WebKit/UIProcess/API/Cocoa/WKWebViewInternal.h

    r224544 r224593  
    149149#endif
    150150
     151#if ENABLE(ATTACHMENT_ELEMENT)
     152- (void)_didRemoveAttachment:(NSString *)identifier;
     153- (void)_didInsertAttachment:(NSString *)identifier;
     154#endif
     155
    151156- (WKPageRef)_pageForTesting;
    152157- (WebKit::WebPageProxy*)_page;

  • TabularUnified trunk/Source/WebKit/UIProcess/Cocoa/PageClientImplCocoa.h

    r224544 r224593  
    3838    void isPlayingAudioWillChange() final;
    3939    void isPlayingAudioDidChange() final;
     40
     41#if ENABLE(ATTACHMENT_ELEMENT)
     42    void didInsertAttachment(const String& identifier) final;
     43    void didRemoveAttachment(const String& identifier) final;
     44#endif
     45
    4046protected:
    4147    WKWebView *m_webView;

  • TabularUnified trunk/Source/WebKit/UIProcess/Cocoa/PageClientImplCocoa.mm

    r224544 r224593  
    2727#import "PageClientImplCocoa.h"
    2828
    29 #import "WKWebViewPrivate.h"
     29#import "WKWebViewInternal.h"
    3030
    3131namespace WebKit {
    … …  
    4444#endif
    4545}
     46
     47#if ENABLE(ATTACHMENT_ELEMENT)
     48
     49void PageClientImplCocoa::didInsertAttachment(const String& identifier)
     50{
     51#if WK_API_ENABLED
     52    [m_webView _didInsertAttachment:identifier];
     53#else
     54    UNUSED_PARAM(identifier);
     55#endif
     56}
     57
     58void PageClientImplCocoa::didRemoveAttachment(const String& identifier)
     59{
     60#if WK_API_ENABLED
     61    [m_webView _didRemoveAttachment:identifier];
     62#else
     63    UNUSED_PARAM(identifier);
     64#endif
     65}
     66
     67#endif
    4668   
    4769}

  • TabularUnified trunk/Source/WebKit/UIProcess/PageClient.h

    r224544 r224593  
    385385#endif
    386386
     387#if ENABLE(ATTACHMENT_ELEMENT)
     388    virtual void didInsertAttachment(const String& identifier) { }
     389    virtual void didRemoveAttachment(const String& identifier) { }
     390#endif
     391
    387392#if PLATFORM(GTK) || PLATFORM(WPE)
    388393    virtual JSGlobalContextRef javascriptGlobalContext() { return nullptr; }

  • TabularUnified trunk/Source/WebKit/UIProcess/WebPageProxy.cpp

    r224544 r224593  
    71147114}
    71157115
     7116void WebPageProxy::didInsertAttachment(const String& identifier)
     7117{
     7118    m_pageClient.didInsertAttachment(identifier);
     7119}
     7120
     7121void WebPageProxy::didRemoveAttachment(const String& identifier)
     7122{
     7123    m_pageClient.didRemoveAttachment(identifier);
     7124}
     7125
    71167126#endif // ENABLE(ATTACHMENT_ELEMENT)
    71177127

  • TabularUnified trunk/Source/WebKit/UIProcess/WebPageProxy.h

    r224544 r224593  
    16381638    void stopAllURLSchemeTasks();
    16391639
     1640#if ENABLE(ATTACHMENT_ELEMENT)
     1641    void didInsertAttachment(const String& identifier);
     1642    void didRemoveAttachment(const String& identifier);
     1643#endif
     1644
    16401645    PageClient& m_pageClient;
    16411646    Ref<API::PageConfiguration> m_configuration;

  • TabularUnified trunk/Source/WebKit/UIProcess/WebPageProxy.messages.in

    r224544 r224593  
    499499
    500500    RequestStorageAccess(String subFrameHost, String topFrameHost, uint64_t contextID)
     501
     502#if ENABLE(ATTACHMENT_ELEMENT)
     503    DidInsertAttachment(String identifier)
     504    DidRemoveAttachment(String identifier)
     505#endif
    501506}

  • TabularUnified trunk/Source/WebKit/WebProcess/WebCoreSupport/WebEditorClient.cpp

    r224544 r224593  
    158158}
    159159
     160#if ENABLE(ATTACHMENT_ELEMENT)
     161
     162void WebEditorClient::didInsertAttachment(const String& identifier)
     163{
     164    m_page->send(Messages::WebPageProxy::DidInsertAttachment(identifier));
     165}
     166
     167void WebEditorClient::didRemoveAttachment(const String& identifier)
     168{
     169    m_page->send(Messages::WebPageProxy::DidRemoveAttachment(identifier));
     170}
     171
     172#endif
     173
    160174void WebEditorClient::didApplyStyle()
    161175{

  • TabularUnified trunk/Source/WebKit/WebProcess/WebCoreSupport/WebEditorClient.h

    r224544 r224593  
    5959    void didApplyStyle() final;
    6060    bool shouldMoveRangeAfterDelete(WebCore::Range*, WebCore::Range*) final;
     61
     62#if ENABLE(ATTACHMENT_ELEMENT)
     63    void didInsertAttachment(const String& identifier) final;
     64    void didRemoveAttachment(const String& identifier) final;
     65#endif
    6166
    6267    void didBeginEditing() final;

  • TabularUnified trunk/Tools/ChangeLog

    r224576 r224593  
     12017-11-08  Wenson Hsieh  <wenson_hsieh@apple.com>
     2
     3        [Attachment Support] Implement delegate hooks for attachment element insertion and removal
     4        https://bugs.webkit.org/show_bug.cgi?id=179016
     5        <rdar://problem/35250890>
     6
     7        Reviewed by Tim Horton.
     8
     9        See WebCore/ChangeLog for more details.
     10
     11        * TestWebKitAPI/Tests/WebKitCocoa/WKAttachmentTests.mm:
     12        (-[AttachmentUpdateObserver init]):
     13        (-[AttachmentUpdateObserver inserted]):
     14        (-[AttachmentUpdateObserver removed]):
     15        (-[AttachmentUpdateObserver _webView:didInsertAttachment:]):
     16        (-[AttachmentUpdateObserver _webView:didRemoveAttachment:]):
     17        (TestWebKitAPI::ObserveAttachmentUpdatesForScope::ObserveAttachmentUpdatesForScope):
     18        (TestWebKitAPI::ObserveAttachmentUpdatesForScope::~ObserveAttachmentUpdatesForScope):
     19        (TestWebKitAPI::ObserveAttachmentUpdatesForScope::observer const):
     20        (TestWebKitAPI::ObserveAttachmentUpdatesForScope::expectAttachmentUpdates):
     21        (-[TestWKWebView _synchronouslyExecuteEditCommand:argument:]):
     22        (-[TestWKWebView expectUpdatesAfterCommand:withArgument:expectedRemovals:expectedInsertions:]):
     23        (TestWebKitAPI::TEST):
     24
    1252017-11-08  Carlos Garcia Campos  <cgarcia@igalia.com>
    226

  • TabularUnified trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/WKAttachmentTests.mm

    r224544 r224593  
    3434#if WK_API_ENABLED
    3535
     36@interface AttachmentUpdateObserver : NSObject <WKUIDelegatePrivate>
     37@property (nonatomic, readonly) NSArray *inserted;
     38@property (nonatomic, readonly) NSArray *removed;
     39@end
     40
     41@implementation AttachmentUpdateObserver {
     42    RetainPtr<NSMutableArray<_WKAttachment *>> _inserted;
     43    RetainPtr<NSMutableArray<_WKAttachment *>> _removed;
     44}
     45
     46- (instancetype)init
     47{
     48    if (self = [super init]) {
     49        _inserted = adoptNS([[NSMutableArray alloc] init]);
     50        _removed = adoptNS([[NSMutableArray alloc] init]);
     51    }
     52    return self;
     53}
     54
     55- (NSArray<_WKAttachment *> *)inserted
     56{
     57    return _inserted.get();
     58}
     59
     60- (NSArray<_WKAttachment *> *)removed
     61{
     62    return _removed.get();
     63}
     64
     65- (void)_webView:(WKWebView *)webView didInsertAttachment:(_WKAttachment *)attachment
     66{
     67    [_inserted addObject:attachment];
     68}
     69
     70- (void)_webView:(WKWebView *)webView didRemoveAttachment:(_WKAttachment *)attachment
     71{
     72    [_removed addObject:attachment];
     73}
     74
     75@end
     76
     77namespace TestWebKitAPI {
     78
     79class ObserveAttachmentUpdatesForScope {
     80public:
     81    ObserveAttachmentUpdatesForScope(TestWKWebView *webView)
     82        : m_webView(webView)
     83    {
     84        m_previousDelegate = retainPtr(webView.UIDelegate);
     85        m_observer = adoptNS([[AttachmentUpdateObserver alloc] init]);
     86        webView.UIDelegate = m_observer.get();
     87    }
     88
     89    ~ObserveAttachmentUpdatesForScope()
     90    {
     91        [m_webView setUIDelegate:m_previousDelegate.get()];
     92    }
     93
     94    AttachmentUpdateObserver *observer() const { return m_observer.get(); }
     95
     96    void expectAttachmentUpdates(NSArray<_WKAttachment *> *removed, NSArray<_WKAttachment *> *inserted)
     97    {
     98        BOOL removedAttachmentsMatch = [observer().removed isEqual:removed];
     99        if (!removedAttachmentsMatch)
     100            NSLog(@"Expected removed attachments: %@ to match %@.", observer().removed, removed);
     101        EXPECT_TRUE(removedAttachmentsMatch);
     102
     103        BOOL insertedAttachmentsMatch = [observer().inserted isEqual:inserted];
     104        if (!insertedAttachmentsMatch)
     105            NSLog(@"Expected inserted attachments: %@ to match %@.", observer().inserted, inserted);
     106        EXPECT_TRUE(insertedAttachmentsMatch);
     107    }
     108
     109private:
     110    RetainPtr<AttachmentUpdateObserver> m_observer;
     111    RetainPtr<TestWKWebView> m_webView;
     112    RetainPtr<id> m_previousDelegate;
     113};
     114
     115}
     116
     117@interface TestWKWebView (AttachmentTesting)
     118@end
     119
    36120static RetainPtr<TestWKWebView> webViewForTestingAttachments()
    37121{
    … …  
    57141
    58142@implementation TestWKWebView (AttachmentTesting)
     143
     144- (BOOL)_synchronouslyExecuteEditCommand:(NSString *)command argument:(NSString *)argument
     145{
     146    __block bool done = false;
     147    __block bool success;
     148    [self _executeEditCommand:command argument:argument completion:^(BOOL completionSuccess) {
     149        done = true;
     150        success = completionSuccess;
     151    }];
     152    TestWebKitAPI::Util::run(&done);
     153    return success;
     154}
    59155
    60156- (_WKAttachment *)synchronouslyInsertAttachmentWithFilename:(NSString *)filename contentType:(NSString *)contentType data:(NSData *)data options:(_WKAttachmentDisplayOptions *)options
    … …  
    73169}
    74170
     171- (void)expectUpdatesAfterCommand:(NSString *)command withArgument:(NSString *)argument expectedRemovals:(NSArray<_WKAttachment *> *)removed expectedInsertions:(NSArray<_WKAttachment *> *)inserted
     172{
     173    TestWebKitAPI::ObserveAttachmentUpdatesForScope observer(self);
     174    EXPECT_TRUE([self _synchronouslyExecuteEditCommand:command argument:argument]);
     175    observer.expectAttachmentUpdates(removed, inserted);
     176}
     177
    75178@end
    76179
    … …  
    80183{
    81184    auto webView = webViewForTestingAttachments();
    82 
    83     // Use the given content type for the attachment element's type.
    84     [webView synchronouslyInsertAttachmentWithFilename:@"foo" contentType:@"text/html" data:testHTMLData() options:nil];
    85     EXPECT_WK_STREQ(@"foo", [webView valueOfAttribute:@"title" forQuerySelector:@"attachment"]);
    86     EXPECT_WK_STREQ(@"text/html", [webView valueOfAttribute:@"type" forQuerySelector:@"attachment"]);
    87     EXPECT_WK_STREQ(@"38 bytes", [webView valueOfAttribute:@"subtitle" forQuerySelector:@"attachment"]);
    88 
    89     // Since no content type is explicitly specified, compute it from the file extension.
    90     [webView _executeEditCommand:@"DeleteBackward" argument:nil completion:nil];
    91     [webView synchronouslyInsertAttachmentWithFilename:@"bar.png" contentType:nil data:testImageData() options:nil];
    92     EXPECT_WK_STREQ(@"bar.png", [webView valueOfAttribute:@"title" forQuerySelector:@"attachment"]);
    93     EXPECT_WK_STREQ(@"image/png", [webView valueOfAttribute:@"type" forQuerySelector:@"attachment"]);
    94     EXPECT_WK_STREQ(@"37 KB", [webView valueOfAttribute:@"subtitle" forQuerySelector:@"attachment"]);
     185    RetainPtr<_WKAttachment> firstAttachment;
     186    RetainPtr<_WKAttachment> secondAttachment;
     187    {
     188        ObserveAttachmentUpdatesForScope observer(webView.get());
     189        // Use the given content type for the attachment element's type.
     190        firstAttachment = retainPtr([webView synchronouslyInsertAttachmentWithFilename:@"foo" contentType:@"text/html" data:testHTMLData() options:nil]);
     191        EXPECT_WK_STREQ(@"foo", [webView valueOfAttribute:@"title" forQuerySelector:@"attachment"]);
     192        EXPECT_WK_STREQ(@"text/html", [webView valueOfAttribute:@"type" forQuerySelector:@"attachment"]);
     193        EXPECT_WK_STREQ(@"38 bytes", [webView valueOfAttribute:@"subtitle" forQuerySelector:@"attachment"]);
     194        observer.expectAttachmentUpdates(@[ ], @[ firstAttachment.get() ]);
     195    }
     196    {
     197        ObserveAttachmentUpdatesForScope scope(webView.get());
     198        // Since no content type is explicitly specified, compute it from the file extension.
     199        [webView _executeEditCommand:@"DeleteBackward" argument:nil completion:nil];
     200        secondAttachment = retainPtr([webView synchronouslyInsertAttachmentWithFilename:@"bar.png" contentType:nil data:testImageData() options:nil]);
     201        EXPECT_WK_STREQ(@"bar.png", [webView valueOfAttribute:@"title" forQuerySelector:@"attachment"]);
     202        EXPECT_WK_STREQ(@"image/png", [webView valueOfAttribute:@"type" forQuerySelector:@"attachment"]);
     203        EXPECT_WK_STREQ(@"37 KB", [webView valueOfAttribute:@"subtitle" forQuerySelector:@"attachment"]);
     204        scope.expectAttachmentUpdates(@[ firstAttachment.get() ], @[ secondAttachment.get() ]);
     205    }
     206}
     207
     208TEST(WKAttachmentTests, AttachmentUpdatesWhenInsertingAndDeletingNewline)
     209{
     210    auto webView = webViewForTestingAttachments();
     211    RetainPtr<_WKAttachment> attachment;
     212    {
     213        ObserveAttachmentUpdatesForScope observer(webView.get());
     214        attachment = retainPtr([webView synchronouslyInsertAttachmentWithFilename:@"foo.txt" contentType:@"text/plain" data:testHTMLData() options:nil]);
     215        observer.expectAttachmentUpdates(@[ ], @[attachment.get()]);
     216    }
     217    [webView expectUpdatesAfterCommand:@"InsertParagraph" withArgument:nil expectedRemovals:@[] expectedInsertions:@[]];
     218    [webView expectUpdatesAfterCommand:@"DeleteBackward" withArgument:nil expectedRemovals:@[] expectedInsertions:@[]];
     219    [webView stringByEvaluatingJavaScript:@"getSelection().collapse(document.body)"];
     220    [webView expectUpdatesAfterCommand:@"InsertParagraph" withArgument:nil expectedRemovals:@[] expectedInsertions:@[]];
     221    [webView expectUpdatesAfterCommand:@"DeleteForward" withArgument:nil expectedRemovals:@[attachment.get()] expectedInsertions:@[]];
     222}
     223
     224TEST(WKAttachmentTests, AttachmentUpdatesWhenUndoingAndRedoing)
     225{
     226    auto webView = webViewForTestingAttachments();
     227    RetainPtr<_WKAttachment> attachment;
     228    {
     229        ObserveAttachmentUpdatesForScope observer(webView.get());
     230        attachment = retainPtr([webView synchronouslyInsertAttachmentWithFilename:@"foo.txt" contentType:@"text/plain" data:testHTMLData() options:nil]);
     231        observer.expectAttachmentUpdates(@[ ], @[attachment.get()]);
     232    }
     233    [webView expectUpdatesAfterCommand:@"Undo" withArgument:nil expectedRemovals:@[attachment.get()] expectedInsertions:@[]];
     234    [webView expectUpdatesAfterCommand:@"Redo" withArgument:nil expectedRemovals:@[] expectedInsertions:@[attachment.get()]];
     235    [webView expectUpdatesAfterCommand:@"DeleteBackward" withArgument:nil expectedRemovals:@[attachment.get()] expectedInsertions:@[]];
     236    [webView expectUpdatesAfterCommand:@"Undo" withArgument:nil expectedRemovals:@[] expectedInsertions:@[attachment.get()]];
     237    [webView expectUpdatesAfterCommand:@"Redo" withArgument:nil expectedRemovals:@[attachment.get()] expectedInsertions:@[]];
     238}
     239
     240TEST(WKAttachmentTests, AttachmentUpdatesWhenChangingFontStyles)
     241{
     242    auto webView = webViewForTestingAttachments();
     243    RetainPtr<_WKAttachment> attachment;
     244    [webView _synchronouslyExecuteEditCommand:@"InsertText" argument:@"Hello"];
     245    {
     246        ObserveAttachmentUpdatesForScope observer(webView.get());
     247        attachment = retainPtr([webView synchronouslyInsertAttachmentWithFilename:@"foo.txt" contentType:@"text/plain" data:testHTMLData() options:nil]);
     248        observer.expectAttachmentUpdates(@[ ], @[attachment.get()]);
     249    }
     250    [webView expectUpdatesAfterCommand:@"InsertText" withArgument:@"World" expectedRemovals:@[] expectedInsertions:@[]];
     251    [webView _synchronouslyExecuteEditCommand:@"SelectAll" argument:nil];
     252    [webView expectUpdatesAfterCommand:@"ToggleBold" withArgument:nil expectedRemovals:@[] expectedInsertions:@[]];
     253    [webView expectUpdatesAfterCommand:@"ToggleItalic" withArgument:nil expectedRemovals:@[] expectedInsertions:@[]];
     254    // FIXME: Additionally test underlining after <https://bugs.webkit.org/show_bug.cgi?id=179431> is addressed.
     255
     256    // Inserting text should delete the current selection, removing the attachment in the process.
     257    [webView expectUpdatesAfterCommand:@"InsertText" withArgument:@"foo" expectedRemovals:@[attachment.get()] expectedInsertions:@[]];
     258}
     259
     260TEST(WKAttachmentTests, AttachmentUpdatesWhenInsertingLists)
     261{
     262    auto webView = webViewForTestingAttachments();
     263    RetainPtr<_WKAttachment> attachment;
     264    {
     265        ObserveAttachmentUpdatesForScope observer(webView.get());
     266        attachment = retainPtr([webView synchronouslyInsertAttachmentWithFilename:@"foo.txt" contentType:@"text/plain" data:testHTMLData() options:nil]);
     267        observer.expectAttachmentUpdates(@[ ], @[attachment.get()]);
     268    }
     269    [webView expectUpdatesAfterCommand:@"InsertOrderedList" withArgument:nil expectedRemovals:@[] expectedInsertions:@[]];
     270    // This edit command behaves more like a "toggle", and will actually break us out of the list we just inserted.
     271    [webView expectUpdatesAfterCommand:@"InsertOrderedList" withArgument:nil expectedRemovals:@[] expectedInsertions:@[]];
     272    [webView expectUpdatesAfterCommand:@"InsertUnorderedList" withArgument:nil expectedRemovals:@[] expectedInsertions:@[]];
     273    [webView expectUpdatesAfterCommand:@"InsertUnorderedList" withArgument:nil expectedRemovals:@[] expectedInsertions:@[]];
     274}
     275
     276TEST(WKAttachmentTests, AttachmentUpdatesWhenInsertingRichMarkup)
     277{
     278    auto webView = webViewForTestingAttachments();
     279    RetainPtr<_WKAttachment> attachment;
     280    {
     281        ObserveAttachmentUpdatesForScope observer(webView.get());
     282        [webView _synchronouslyExecuteEditCommand:@"InsertHTML" argument:@"<div><strong><attachment title='a' webkitattachmentid='a06fec41-9aa0-4c2c-ba3a-0149b54aad99'></attachment></strong></div>"];
     283        attachment = observer.observer().inserted[0];
     284        observer.expectAttachmentUpdates(@[ ], @[attachment.get()]);
     285    }
     286    {
     287        ObserveAttachmentUpdatesForScope observer(webView.get());
     288        [webView stringByEvaluatingJavaScript:@"document.querySelector('attachment').remove()"];
     289        [webView waitForNextPresentationUpdate];
     290        observer.expectAttachmentUpdates(@[attachment.get()], @[ ]);
     291    }
    95292}
    96293
Note: See TracChangeset for help on using the changeset viewer.