[Services with UI] Action menu does not appear if selection includes both text and...
authorenrica@apple.com <enrica@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 7 Aug 2014 23:08:38 +0000 (23:08 +0000)
committerenrica@apple.com <enrica@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 7 Aug 2014 23:08:38 +0000 (23:08 +0000)
https://bugs.webkit.org/show_bug.cgi?id=135731
<rdar://problem/17837491>

Reviewed by Brady Eidson.

Source/WebCore:

When collecting selection rects via SelectionRectGatherer we should also note if the selection
contains non text elements. This way the Notifier class can send that information to ServicesOverlayController
to properly handle the highlight for the service.

* editing/SelectionRectGatherer.cpp:
(WebCore::SelectionRectGatherer::SelectionRectGatherer):
(WebCore::SelectionRectGatherer::Notifier::~Notifier):
(WebCore::SelectionRectGatherer::clearAndCreateNotifier):
* editing/SelectionRectGatherer.h:
(WebCore::SelectionRectGatherer::setTextOnly):
(WebCore::SelectionRectGatherer::isTextOnly):
* page/EditorClient.h:
(WebCore::EditorClient::selectionRectsDidChange):
* rendering/RenderView.cpp:
(WebCore::RenderView::applySubtreeSelection):

Source/WebKit2:

Adding a new setting to ServicesController to communicate to the WebProcess if
there are services installed that can handle a combination of text and images.
This way ServicesOverlayController can decide if it appropriate to show the hightlight
based on the type of selection (text only or non text only). This information is retrieved
when the selection rects are collected by SelectionGatherer and used by
SelectionGatherer::Notifier to communicate the selection change.

* Shared/WebProcessCreationParameters.cpp:
(WebKit::WebProcessCreationParameters::WebProcessCreationParameters):
* Shared/WebProcessCreationParameters.h:
* UIProcess/mac/ServicesController.h:
(WebKit::ServicesController::hasRichContentServices):
* UIProcess/mac/ServicesController.mm:
(WebKit::ServicesController::ServicesController):
(WebKit::ServicesController::refreshExistingServices):
* WebProcess/WebCoreSupport/WebEditorClient.cpp:
(WebKit::WebEditorClient::selectionRectsDidChange):
* WebProcess/WebCoreSupport/WebEditorClient.h:
* WebProcess/WebPage/ServicesOverlayController.h:
* WebProcess/WebPage/mac/ServicesOverlayController.mm:
(WebKit::ServicesOverlayController::ServicesOverlayController):
(WebKit::ServicesOverlayController::selectionRectsDidChange):
(WebKit::ServicesOverlayController::drawSelectionHighlight):
* WebProcess/WebProcess.cpp:
(WebKit::WebProcess::WebProcess):
(WebKit::WebProcess::initializeWebProcess):
(WebKit::WebProcess::setEnabledServices):
* WebProcess/WebProcess.h:
(WebKit::WebProcess::hasRichContentServices):
* WebProcess/WebProcess.messages.in:

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@172301 268f45cc-cd09-0410-ab3c-d52691b4dbfc

17 files changed:
Source/WebCore/ChangeLog
Source/WebCore/editing/SelectionRectGatherer.cpp
Source/WebCore/editing/SelectionRectGatherer.h
Source/WebCore/page/EditorClient.h
Source/WebCore/rendering/RenderView.cpp
Source/WebKit2/ChangeLog
Source/WebKit2/Shared/WebProcessCreationParameters.cpp
Source/WebKit2/Shared/WebProcessCreationParameters.h
Source/WebKit2/UIProcess/mac/ServicesController.h
Source/WebKit2/UIProcess/mac/ServicesController.mm
Source/WebKit2/WebProcess/WebCoreSupport/WebEditorClient.cpp
Source/WebKit2/WebProcess/WebCoreSupport/WebEditorClient.h
Source/WebKit2/WebProcess/WebPage/ServicesOverlayController.h
Source/WebKit2/WebProcess/WebPage/mac/ServicesOverlayController.mm
Source/WebKit2/WebProcess/WebProcess.cpp
Source/WebKit2/WebProcess/WebProcess.h
Source/WebKit2/WebProcess/WebProcess.messages.in

index 911741a20719fe5805f248f9ed6db326cd370e6b..8117fdc1efbcba243267800ab1098fc65538aaba 100644 (file)
@@ -1,3 +1,27 @@
+2014-08-07  Enrica Casucci  <enrica@apple.com>
+
+        [Services with UI] Action menu does not appear if selection includes both text and an image.
+        https://bugs.webkit.org/show_bug.cgi?id=135731
+        <rdar://problem/17837491>
+
+        Reviewed by Brady Eidson.
+
+        When collecting selection rects via SelectionRectGatherer we should also note if the selection
+        contains non text elements. This way the Notifier class can send that information to ServicesOverlayController
+        to properly handle the highlight for the service.
+
+        * editing/SelectionRectGatherer.cpp:
+        (WebCore::SelectionRectGatherer::SelectionRectGatherer):
+        (WebCore::SelectionRectGatherer::Notifier::~Notifier):
+        (WebCore::SelectionRectGatherer::clearAndCreateNotifier):
+        * editing/SelectionRectGatherer.h:
+        (WebCore::SelectionRectGatherer::setTextOnly):
+        (WebCore::SelectionRectGatherer::isTextOnly):
+        * page/EditorClient.h:
+        (WebCore::EditorClient::selectionRectsDidChange):
+        * rendering/RenderView.cpp:
+        (WebCore::RenderView::applySubtreeSelection):
+
 2014-08-07  Andy Estes  <aestes@apple.com>
 
         [Mac] Parental Controls content filter is mistakenly enabled for HTTP responses
index 4920b34fd528192bf52a54ec8bdc7a27546f5f54..a65f7be1a0caf92008c143ef508271bad452d284 100644 (file)
@@ -37,6 +37,7 @@ namespace WebCore {
 
 SelectionRectGatherer::SelectionRectGatherer(RenderView& renderView)
     : m_renderView(renderView)
+    , m_isTextOnly(true)
 {
 }
 
@@ -70,13 +71,14 @@ SelectionRectGatherer::Notifier::Notifier(SelectionRectGatherer& gatherer)
 SelectionRectGatherer::Notifier::~Notifier()
 {
     if (EditorClient* client = m_gatherer.m_renderView.view().frame().editor().client())
-        client->selectionRectsDidChange(m_gatherer.m_rects, m_gatherer.m_gapRects);
+        client->selectionRectsDidChange(m_gatherer.m_rects, m_gatherer.m_gapRects, m_gatherer.isTextOnly());
 }
 
 std::unique_ptr<SelectionRectGatherer::Notifier> SelectionRectGatherer::clearAndCreateNotifier()
 {
     m_rects.clear();
     m_gapRects.clear();
+    m_isTextOnly = true;
 
     return std::make_unique<Notifier>(*this);
 }
index d36e47f4bb95828d163fd54d679b6bbd5e6388d7..bcddc9bf96b2b89cb6cbd5672940ac92be935e0f 100644 (file)
@@ -47,6 +47,8 @@ public:
 
     void addRect(RenderLayerModelObject *repaintContainer, const LayoutRect&);
     void addGapRects(RenderLayerModelObject *repaintContainer, const GapRects&);
+    void setTextOnly(bool isTextOnly) { m_isTextOnly = isTextOnly; }
+    bool isTextOnly() const { return m_isTextOnly; }
 
     class Notifier {
         WTF_MAKE_NONCOPYABLE(Notifier);
@@ -66,6 +68,7 @@ private:
     // All rects are in RenderView coordinates.
     Vector<LayoutRect> m_rects;
     Vector<GapRects> m_gapRects;
+    bool m_isTextOnly;
 };
 
 } // namespace WebCore
index d2f5ced2e887328022000802766c0542f437099c..de72bd0713b28242d9e2fa3725fc852a2b181e2c 100644 (file)
@@ -183,7 +183,7 @@ public:
 
 #if ENABLE(SERVICE_CONTROLS) || ENABLE(TELEPHONE_NUMBER_DETECTION)
     virtual void selectedTelephoneNumberRangesChanged(const Vector<RefPtr<Range>>&) { }
-    virtual void selectionRectsDidChange(const Vector<LayoutRect>&, const Vector<GapRects>&) { }
+    virtual void selectionRectsDidChange(const Vector<LayoutRect>&, const Vector<GapRects>&, bool) { }
 #endif
 
     // Support for global selections, used on platforms like the X Window System that treat
index 9670ffaa2e050034ded915cfa7824ccf6504cfa3..3b0b428c69882babc90fbacc6d031fb1f9cecf3f 100644 (file)
@@ -1016,6 +1016,8 @@ void RenderView::applySubtreeSelection(SelectionSubtreeRoot& root, RenderObject*
 #if ENABLE(SERVICE_CONTROLS)
             for (auto& rect : selectionInfo->collectedSelectionRects())
                 m_selectionRectGatherer.addRect(selectionInfo->repaintContainer(), rect);
+            if (!o->isTextOrLineBreak())
+                m_selectionRectGatherer.setTextOnly(false);
 #endif
 
             newSelectedObjects.set(o, WTF::move(selectionInfo));
index 56a81b6995496407fc3b55d2176e6b933cc742e1..e58002283e916033dde19066f3786aebf67f89e6 100644 (file)
@@ -1,3 +1,42 @@
+2014-08-07  Enrica Casucci  <enrica@apple.com>
+
+        [Services with UI] Action menu does not appear if selection includes both text and an image.
+        https://bugs.webkit.org/show_bug.cgi?id=135731
+        <rdar://problem/17837491>
+
+        Reviewed by Brady Eidson.
+
+        Adding a new setting to ServicesController to communicate to the WebProcess if
+        there are services installed that can handle a combination of text and images.
+        This way ServicesOverlayController can decide if it appropriate to show the hightlight
+        based on the type of selection (text only or non text only). This information is retrieved
+        when the selection rects are collected by SelectionGatherer and used by
+        SelectionGatherer::Notifier to communicate the selection change.
+
+        * Shared/WebProcessCreationParameters.cpp:
+        (WebKit::WebProcessCreationParameters::WebProcessCreationParameters):
+        * Shared/WebProcessCreationParameters.h:
+        * UIProcess/mac/ServicesController.h:
+        (WebKit::ServicesController::hasRichContentServices):
+        * UIProcess/mac/ServicesController.mm:
+        (WebKit::ServicesController::ServicesController):
+        (WebKit::ServicesController::refreshExistingServices):
+        * WebProcess/WebCoreSupport/WebEditorClient.cpp:
+        (WebKit::WebEditorClient::selectionRectsDidChange):
+        * WebProcess/WebCoreSupport/WebEditorClient.h:
+        * WebProcess/WebPage/ServicesOverlayController.h:
+        * WebProcess/WebPage/mac/ServicesOverlayController.mm:
+        (WebKit::ServicesOverlayController::ServicesOverlayController):
+        (WebKit::ServicesOverlayController::selectionRectsDidChange):
+        (WebKit::ServicesOverlayController::drawSelectionHighlight):
+        * WebProcess/WebProcess.cpp:
+        (WebKit::WebProcess::WebProcess):
+        (WebKit::WebProcess::initializeWebProcess):
+        (WebKit::WebProcess::setEnabledServices):
+        * WebProcess/WebProcess.h:
+        (WebKit::WebProcess::hasRichContentServices):
+        * WebProcess/WebProcess.messages.in:
+
 2014-08-07  Ryuan Choi  <ryuan.choi@samsung.com>
 
         [EFL] Fix several warnings of doxygen
index e95f0bc3296691e945bc409ed5675ddbe6ba8473..e6889824dc173857b8064d2205ece7fd5a955f74 100644 (file)
@@ -51,6 +51,7 @@ WebProcessCreationParameters::WebProcessCreationParameters()
 #if ENABLE(SERVICE_CONTROLS)
     , hasImageServices(false)
     , hasSelectionServices(false)
+    , hasRichContentServices(false)
 #endif
 {
 }
index 0af54de17cfb02b3ef96234ddfb2b56e67ba25c8..26cb810aec800c5c9ae883e2b350b74c5562ffed 100644 (file)
@@ -168,6 +168,7 @@ struct WebProcessCreationParameters {
 #if ENABLE(SERVICE_CONTROLS)
     bool hasImageServices;
     bool hasSelectionServices;
+    bool hasRichContentServices;
 #endif
 };
 
index c6b3221d336affad7e7008f78710e91026652f3e..aa3386327e9cc9dfbd561a13b05b5ad538194b0b 100644 (file)
@@ -45,6 +45,7 @@ public:
 
     bool hasImageServices() const { return m_hasImageServices; }
     bool hasSelectionServices() const { return m_hasSelectionServices; }
+    bool hasRichContentServices() const { return m_hasRichContentServices; }
 
     void refreshExistingServices(WebContext*);
 
@@ -58,6 +59,7 @@ private:
 
     bool m_hasImageServices;
     bool m_hasSelectionServices;
+    bool m_hasRichContentServices;
 
     HashSet<RefPtr<WebContext>> m_contextsToNotify;
 };
index 112b879507739c10ce8139621201ad4978b374ce..07f90243fbd40bf2f81df68692a7ca8995136a96 100644 (file)
@@ -61,6 +61,7 @@ ServicesController::ServicesController()
     , m_isRefreshing(false)
     , m_hasImageServices(false)
     , m_hasSelectionServices(false)
+    , m_hasRichContentServices(false)
 {
     refreshExistingServices();
 }
@@ -93,14 +94,30 @@ void ServicesController::refreshExistingServices()
 
         bool hasSelectionServices = picker.get().menu;
 
+        static NSAttributedString *attributedStringWithRichContent;
+        if (!attributedStringWithRichContent) {
+            NSTextAttachment *attachment = [[NSTextAttachment alloc] init];
+            NSTextAttachmentCell *cell = [[NSTextAttachmentCell alloc] initImageCell:image.get()];
+            [attachment setAttachmentCell:cell];
+            NSMutableAttributedString *richString = (NSMutableAttributedString *)[NSMutableAttributedString attributedStringWithAttachment:attachment];
+            [richString appendAttributedString: attributedString];
+            attributedStringWithRichContent = [richString retain];
+        }
+
+        picker = adoptNS([[NSSharingServicePicker alloc] initWithItems:@[ attributedStringWithRichContent ]]);
+        [picker setStyle:NSSharingServicePickerStyleTextSelection];
+
+        bool hasRichContentServices = picker.get().menu;
+        
         dispatch_async(dispatch_get_main_queue(), ^{
-            bool notifyContexts = (hasImageServices != m_hasImageServices) || (hasSelectionServices != m_hasSelectionServices);
+            bool notifyContexts = (hasImageServices != m_hasImageServices) || (hasSelectionServices != m_hasSelectionServices) || (hasRichContentServices != m_hasRichContentServices);
             m_hasSelectionServices = hasSelectionServices;
             m_hasImageServices = hasImageServices;
+            m_hasRichContentServices = hasRichContentServices;
 
             if (notifyContexts) {
                 for (const RefPtr<WebContext>& context : m_contextsToNotify)
-                    context->sendToAllProcesses(Messages::WebProcess::SetEnabledServices(m_hasImageServices, m_hasSelectionServices));
+                    context->sendToAllProcesses(Messages::WebProcess::SetEnabledServices(m_hasImageServices, m_hasSelectionServices, m_hasRichContentServices));
             }
 
             m_contextsToNotify.clear();
index 140761badd60141a7390315bb58c6dadecf742c1..57e19ee21226e5bdcd036424fa3478abc9a16d09 100644 (file)
@@ -538,11 +538,11 @@ void WebEditorClient::selectedTelephoneNumberRangesChanged(const Vector<RefPtr<R
     m_page->servicesOverlayController().selectedTelephoneNumberRangesChanged(ranges);
 #endif
 }
-void WebEditorClient::selectionRectsDidChange(const Vector<LayoutRect>& rects, const Vector<GapRects>& gapRects)
+void WebEditorClient::selectionRectsDidChange(const Vector<LayoutRect>& rects, const Vector<GapRects>& gapRects, bool isTextOnly)
 {
 #if PLATFORM(MAC)
     if (m_page->serviceControlsEnabled())
-        m_page->servicesOverlayController().selectionRectsDidChange(rects, gapRects);
+        m_page->servicesOverlayController().selectionRectsDidChange(rects, gapRects, isTextOnly);
 #endif
 }
 #endif // ENABLE(SERVICE_CONTROLS) && ENABLE(TELEPHONE_NUMBER_DETECTION)
index 09687848e419bbdb8b208024a70511cd5e4cdb4a..a33f406004adc8612ab52347c46431843f1ec952 100644 (file)
@@ -170,7 +170,7 @@ private:
 
 #if ENABLE(TELEPHONE_NUMBER_DETECTION) || ENABLE(SERVICE_CONTROLS)
     virtual void selectedTelephoneNumberRangesChanged(const Vector<RefPtr<WebCore::Range>>&) override;
-    virtual void selectionRectsDidChange(const Vector<WebCore::LayoutRect>&, const Vector<WebCore::GapRects>&) override;
+    virtual void selectionRectsDidChange(const Vector<WebCore::LayoutRect>&, const Vector<WebCore::GapRects>&, bool isTextOnly) override;
 #endif
 
     WebPage* m_page;
index 2304782c5b5ba3b3012fd592cc7f583fc6b49bd9..b4f3b1ba08efcd498aa55cc649a046d1f22d84c5 100644 (file)
@@ -63,7 +63,7 @@ public:
     ~ServicesOverlayController();
 
     void selectedTelephoneNumberRangesChanged(const Vector<RefPtr<WebCore::Range>>&);
-    void selectionRectsDidChange(const Vector<WebCore::LayoutRect>&, const Vector<WebCore::GapRects>&);
+    void selectionRectsDidChange(const Vector<WebCore::LayoutRect>&, const Vector<WebCore::GapRects>&, bool isTextOnly);
 
 private:
     void createOverlayIfNeeded();
@@ -95,6 +95,7 @@ private:
     
     Vector<WebCore::LayoutRect> m_currentSelectionRects;
     RetainPtr<DDHighlightRef> m_selectionHighlight;
+    bool m_isTextOnly;
     std::chrono::steady_clock::time_point m_lastSelectionChangeTime;
     std::chrono::steady_clock::time_point m_lastHoveredHighlightChangeTime;
 
index 950ee6adab2309f953e86db06b0ad8a072184e17..79b2372e44d0f5bdc13291cde908502081ce5f00 100644 (file)
@@ -76,6 +76,7 @@ static IntRect textQuadsToBoundingRectForRange(Range& range)
 ServicesOverlayController::ServicesOverlayController(WebPage& webPage)
     : m_webPage(&webPage)
     , m_servicesOverlay(nullptr)
+    , m_isTextOnly(false)
     , m_repaintHighlightTimer(this, &ServicesOverlayController::repaintHighlightTimerFired)
 {
 }
@@ -211,11 +212,12 @@ static void compactRectsWithGapRects(Vector<LayoutRect>& rects, const Vector<Gap
     }
 }
 
-void ServicesOverlayController::selectionRectsDidChange(const Vector<LayoutRect>& rects, const Vector<GapRects>& gapRects)
+void ServicesOverlayController::selectionRectsDidChange(const Vector<LayoutRect>& rects, const Vector<GapRects>& gapRects, bool isTextOnly)
 {
 #if __MAC_OS_X_VERSION_MIN_REQUIRED > 1090
     clearSelectionHighlight();
     m_currentSelectionRects = rects;
+    m_isTextOnly = isTextOnly;
 
     m_lastSelectionChangeTime = std::chrono::steady_clock::now();
 
@@ -271,7 +273,7 @@ void ServicesOverlayController::drawSelectionHighlight(WebCore::GraphicsContext&
 {
     // It's possible to end up drawing the selection highlight before we've actually received the selection rects.
     // If that happens we'll end up here again once we have the rects.
-    if (m_currentSelectionRects.isEmpty())
+    if (m_currentSelectionRects.isEmpty() || (!WebProcess::shared().hasRichContentServices() && !m_isTextOnly))
         return;
 
     // If there are no installed selection services and we have no phone numbers detected, then we have nothing to draw.
index 7f3c120006b93ca72d6df483979d0462ca3a0754..2ca241f4dbcea54fe3b8e5dceb31ac53117da4ac 100644 (file)
@@ -174,6 +174,7 @@ WebProcess::WebProcess()
 #if ENABLE(SERVICE_CONTROLS)
     , m_hasImageServices(false)
     , m_hasSelectionServices(false)
+    , m_hasRichContentServices(false)
 #endif
     , m_nonVisibleProcessCleanupTimer(this, &WebProcess::nonVisibleProcessCleanupTimerFired)
 {
@@ -364,7 +365,7 @@ void WebProcess::initializeWebProcess(const WebProcessCreationParameters& parame
     setMemoryCacheDisabled(parameters.memoryCacheDisabled);
 
 #if ENABLE(SERVICE_CONTROLS)
-    setEnabledServices(parameters.hasImageServices, parameters.hasSelectionServices);
+    setEnabledServices(parameters.hasImageServices, parameters.hasSelectionServices, parameters.hasRichContentServices);
 #endif
 
 #if ENABLE(REMOTE_INSPECTOR)
@@ -1251,10 +1252,11 @@ void WebProcess::setMemoryCacheDisabled(bool disabled)
 }
 
 #if ENABLE(SERVICE_CONTROLS)
-void WebProcess::setEnabledServices(bool hasImageServices, bool hasSelectionServices)
+void WebProcess::setEnabledServices(bool hasImageServices, bool hasSelectionServices, bool hasRichContentServices)
 {
     m_hasImageServices = hasImageServices;
     m_hasSelectionServices = hasSelectionServices;
+    m_hasRichContentServices = hasRichContentServices;
 }
 #endif
 
index 5e389e4529b2c3bd2d4b8f073ec158e2f317f023..58febbc9db98f72006f0ee545321dd63aee4752f 100644 (file)
@@ -189,6 +189,7 @@ public:
 #if ENABLE(SERVICE_CONTROLS)
     bool hasImageServices() const { return m_hasImageServices; }
     bool hasSelectionServices() const { return m_hasSelectionServices; }
+    bool hasRichContentServices() const { return m_hasRichContentServices; }
 #endif
 
 private:
@@ -252,7 +253,7 @@ private:
     void setMemoryCacheDisabled(bool);
 
 #if ENABLE(SERVICE_CONTROLS)
-    void setEnabledServices(bool hasImageServices, bool hasSelectionServices);
+    void setEnabledServices(bool hasImageServices, bool hasSelectionServices, bool hasRichContentServices);
 #endif
 
     void postInjectedBundleMessage(const IPC::DataReference& messageData);
@@ -339,6 +340,7 @@ private:
 #if ENABLE(SERVICE_CONTROLS)
     bool m_hasImageServices;
     bool m_hasSelectionServices;
+    bool m_hasRichContentServices;
 #endif
 
     HashSet<uint64_t> m_pagesInWindows;
index 4312f566cd5f33c97a680057c5963eacf8db7751..ff7116fa172e3fb2ac96dc3a8897ea2ba30cd53f 100644 (file)
@@ -90,7 +90,7 @@ messages -> WebProcess LegacyReceiver {
     SetMemoryCacheDisabled(bool disabled);
 
 #if ENABLE(SERVICE_CONTROLS)
-    SetEnabledServices(bool hasImageServices, bool hasSelectionServices)
+    SetEnabledServices(bool hasImageServices, bool hasSelectionServices, bool hasRichContentServices)
 #endif
 
     ProcessWillSuspend()