- Revision
- 278331
- Author
- [email protected]
- Date
- 2021-06-01 15:36:56 -0700 (Tue, 01 Jun 2021)
Log Message
[macOS] Data detector highlights should appear when hovering inside image overlays
https://bugs.webkit.org/show_bug.cgi?id=226507
Reviewed by Tim Horton.
Refactor some logic in ImageOverlayController, such that the overlay is installed if _either_ selection painting
is required, or data detector highlights for the image overlay host element exist; additionally, add plumbing to
allow the ImageOverlayController to install a page overlay when the element under the mouse is inside an image
overlay.
* dom/Document.cpp:
(WebCore::Document::willBeRemovedFromFrame):
Drive-by fix: use `imageOverlayControllerIfExists` instead of `imageOverlayController` to avoid unnecessarily
creating a new ImageOverlayController when detaching a Document.
* page/EventHandler.cpp:
(WebCore::EventHandler::clear):
Refactor logic for clearing out `m_elementUnderMouse` into a separate helper, and call it from these two places.
This new helper clears out `m_elementUnderMouse` and additionally notifies the page's image overlay controller
(only if it has already been constructed).
(WebCore::EventHandler::updateMouseEventTargetNode):
Call out to the page's image overlay controller when changing `m_elementUnderMouse`.
(WebCore::EventHandler::clearElementUnderMouse):
* page/EventHandler.h:
* page/ImageOverlayController.cpp:
(WebCore::ImageOverlayController::selectionQuadsDidChange):
Refactor ImageOverlayController so that it only installs its page overlay if either:
1. Selection painting is needed for selected text inside an image overlay, or...
2. The cursor is over an image overlay host element with data detection results.
To achieve this, we maintain a weak pointer to the image overlay host that contains the selection separately
from the weak pointer to the image overlay host that contains data detectors, and is the element currently under
the mouse cursor.
Drive-by fix: also ignore selection updates due to temporary selections triggered as a result of gathering
dictionary popup info.
(WebCore::ImageOverlayController::documentDetached):
(WebCore::ImageOverlayController::uninstallPageOverlay):
(WebCore::ImageOverlayController::uninstallPageOverlayIfNeeded):
Split this into two functions: `uninstallPageOverlayIfNeeded`, which removes and destroys the PageOverlay if
it is no longer needed (i.e. the overlay is required for neither selection painting nor data detectors).
(WebCore::ImageOverlayController::willMoveToPage):
(WebCore::ImageOverlayController::drawRect):
(WebCore::ImageOverlayController::elementUnderMouseDidChange):
* page/ImageOverlayController.h:
Rename `m_imageOverlayBounds` to `m_selectionClipRect` to make it clear that this is only used during selection
painting, and rename `m_overlaySelectionQuads` to just `m_selectionQuads` for conciseness.
* page/Page.h:
(WebCore::Page::imageOverlayControllerIfExists):
Add a version of this getter that does not initialize the ImageOverlayController if it didn't already exist. See
call sites in EventHandler and Document.
* page/mac/ImageOverlayControllerMac.mm:
(WebCore::ImageOverlayController::clearDataDetectorHighlights):
Additionally clear out `m_hostElementForDataDetectors`.
(WebCore::ImageOverlayController::elementUnderMouseDidChange):
Update data detector highlights whenever the element under the mouse is over content inside an image overlay.
Note that we effectively ignore this method call in the case where we're clearing out `m_elementUnderMouse` for
a different Document than the one containing `m_hostElementForDataDetectors`, which prevents us from erroneously
hiding data detectors when `m_elementUnderMouse` is removed or otherwise cleared out in a different Document.
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (278330 => 278331)
--- trunk/Source/WebCore/ChangeLog 2021-06-01 22:36:13 UTC (rev 278330)
+++ trunk/Source/WebCore/ChangeLog 2021-06-01 22:36:56 UTC (rev 278331)
@@ -1,3 +1,81 @@
+2021-06-01 Wenson Hsieh <[email protected]>
+
+ [macOS] Data detector highlights should appear when hovering inside image overlays
+ https://bugs.webkit.org/show_bug.cgi?id=226507
+
+ Reviewed by Tim Horton.
+
+ Refactor some logic in ImageOverlayController, such that the overlay is installed if _either_ selection painting
+ is required, or data detector highlights for the image overlay host element exist; additionally, add plumbing to
+ allow the ImageOverlayController to install a page overlay when the element under the mouse is inside an image
+ overlay.
+
+ * dom/Document.cpp:
+ (WebCore::Document::willBeRemovedFromFrame):
+
+ Drive-by fix: use `imageOverlayControllerIfExists` instead of `imageOverlayController` to avoid unnecessarily
+ creating a new ImageOverlayController when detaching a Document.
+
+ * page/EventHandler.cpp:
+ (WebCore::EventHandler::clear):
+
+ Refactor logic for clearing out `m_elementUnderMouse` into a separate helper, and call it from these two places.
+ This new helper clears out `m_elementUnderMouse` and additionally notifies the page's image overlay controller
+ (only if it has already been constructed).
+
+ (WebCore::EventHandler::updateMouseEventTargetNode):
+
+ Call out to the page's image overlay controller when changing `m_elementUnderMouse`.
+
+ (WebCore::EventHandler::clearElementUnderMouse):
+ * page/EventHandler.h:
+ * page/ImageOverlayController.cpp:
+ (WebCore::ImageOverlayController::selectionQuadsDidChange):
+
+ Refactor ImageOverlayController so that it only installs its page overlay if either:
+ 1. Selection painting is needed for selected text inside an image overlay, or...
+ 2. The cursor is over an image overlay host element with data detection results.
+
+ To achieve this, we maintain a weak pointer to the image overlay host that contains the selection separately
+ from the weak pointer to the image overlay host that contains data detectors, and is the element currently under
+ the mouse cursor.
+
+ Drive-by fix: also ignore selection updates due to temporary selections triggered as a result of gathering
+ dictionary popup info.
+
+ (WebCore::ImageOverlayController::documentDetached):
+ (WebCore::ImageOverlayController::uninstallPageOverlay):
+ (WebCore::ImageOverlayController::uninstallPageOverlayIfNeeded):
+
+ Split this into two functions: `uninstallPageOverlayIfNeeded`, which removes and destroys the PageOverlay if
+ it is no longer needed (i.e. the overlay is required for neither selection painting nor data detectors).
+
+ (WebCore::ImageOverlayController::willMoveToPage):
+ (WebCore::ImageOverlayController::drawRect):
+ (WebCore::ImageOverlayController::elementUnderMouseDidChange):
+ * page/ImageOverlayController.h:
+
+ Rename `m_imageOverlayBounds` to `m_selectionClipRect` to make it clear that this is only used during selection
+ painting, and rename `m_overlaySelectionQuads` to just `m_selectionQuads` for conciseness.
+
+ * page/Page.h:
+ (WebCore::Page::imageOverlayControllerIfExists):
+
+ Add a version of this getter that does not initialize the ImageOverlayController if it didn't already exist. See
+ call sites in EventHandler and Document.
+
+ * page/mac/ImageOverlayControllerMac.mm:
+ (WebCore::ImageOverlayController::clearDataDetectorHighlights):
+
+ Additionally clear out `m_hostElementForDataDetectors`.
+
+ (WebCore::ImageOverlayController::elementUnderMouseDidChange):
+
+ Update data detector highlights whenever the element under the mouse is over content inside an image overlay.
+ Note that we effectively ignore this method call in the case where we're clearing out `m_elementUnderMouse` for
+ a different Document than the one containing `m_hostElementForDataDetectors`, which prevents us from erroneously
+ hiding data detectors when `m_elementUnderMouse` is removed or otherwise cleared out in a different Document.
+
2021-06-01 Chris Dumez <[email protected]>
Fix unsafe access to m_upload in XMLHttpRequest::virtualHasPendingActivity()
Modified: trunk/Source/WebCore/dom/Document.cpp (278330 => 278331)
--- trunk/Source/WebCore/dom/Document.cpp 2021-06-01 22:36:13 UTC (rev 278330)
+++ trunk/Source/WebCore/dom/Document.cpp 2021-06-01 22:36:56 UTC (rev 278331)
@@ -2621,7 +2621,8 @@
#if ENABLE(POINTER_LOCK)
page->pointerLockController().documentDetached(*this);
#endif
- page->imageOverlayController().documentDetached(*this);
+ if (auto* imageOverlayController = page->imageOverlayControllerIfExists())
+ imageOverlayController->documentDetached(*this);
if (auto* validationMessageClient = page->validationMessageClient())
validationMessageClient->documentDetached(*this);
}
Modified: trunk/Source/WebCore/page/EventHandler.cpp (278330 => 278331)
--- trunk/Source/WebCore/page/EventHandler.cpp 2021-06-01 22:36:13 UTC (rev 278330)
+++ trunk/Source/WebCore/page/EventHandler.cpp 2021-06-01 22:36:56 UTC (rev 278331)
@@ -65,6 +65,7 @@
#include "HitTestRequest.h"
#include "HitTestResult.h"
#include "Image.h"
+#include "ImageOverlayController.h"
#include "InspectorInstrumentation.h"
#include "KeyboardEvent.h"
#include "Logging.h"
@@ -370,7 +371,7 @@
m_imageExtractionTimer.stop();
#endif
m_resizeLayer = nullptr;
- m_elementUnderMouse = nullptr;
+ clearElementUnderMouse();
m_lastElementUnderMouse = nullptr;
m_lastMouseMoveEventSubframe = nullptr;
m_lastScrollbarUnderMouse = nullptr;
@@ -2547,6 +2548,9 @@
}
#endif
+ if (auto* page = m_frame.page())
+ page->imageOverlayController().elementUnderMouseDidChange(m_frame, m_elementUnderMouse.get());
+
ASSERT_IMPLIES(m_elementUnderMouse, &m_elementUnderMouse->document() == m_frame.document());
ASSERT_IMPLIES(m_lastElementUnderMouse, &m_lastElementUnderMouse->document() == m_frame.document());
@@ -2606,7 +2610,7 @@
#if ENABLE(IMAGE_EXTRACTION)
m_imageExtractionTimer.stop();
#endif
- m_elementUnderMouse = nullptr;
+ clearElementUnderMouse();
}
m_lastElementUnderMouse = m_elementUnderMouse;
@@ -2613,6 +2617,24 @@
}
}
+void EventHandler::clearElementUnderMouse()
+{
+ if (!m_elementUnderMouse)
+ return;
+
+ m_elementUnderMouse = nullptr;
+
+ auto* page = m_frame.page();
+ if (!page)
+ return;
+
+ auto* imageOverlayController = page->imageOverlayControllerIfExists();
+ if (!imageOverlayController)
+ return;
+
+ imageOverlayController->elementUnderMouseDidChange(m_frame, nullptr);
+}
+
void EventHandler::notifyScrollableAreasOfMouseEvents(const AtomString& eventType, Element* lastElementUnderMouse, Element* elementUnderMouse)
{
auto* frameView = m_frame.view();
Modified: trunk/Source/WebCore/page/EventHandler.h (278330 => 278331)
--- trunk/Source/WebCore/page/EventHandler.h 2021-06-01 22:36:13 UTC (rev 278330)
+++ trunk/Source/WebCore/page/EventHandler.h 2021-06-01 22:36:56 UTC (rev 278331)
@@ -519,6 +519,7 @@
#endif
void clearLatchedState();
+ void clearElementUnderMouse();
bool shouldSendMouseEventsToInactiveWindows() const;
Modified: trunk/Source/WebCore/page/ImageOverlayController.cpp (278330 => 278331)
--- trunk/Source/WebCore/page/ImageOverlayController.cpp 2021-06-01 22:36:13 UTC (rev 278330)
+++ trunk/Source/WebCore/page/ImageOverlayController.cpp 2021-06-01 22:36:56 UTC (rev 278331)
@@ -58,9 +58,14 @@
if (!m_page || !m_page->chrome().client().needsImageOverlayControllerForSelectionPainting())
return;
- if (frame.editor().ignoreSelectionChanges())
+ if (frame.editor().ignoreSelectionChanges() || frame.editor().isGettingDictionaryPopupInfo())
return;
+ m_hostElementForSelection = nullptr;
+ m_selectionQuads.clear();
+ m_selectionBackgroundColor = Color::transparentBlack;
+ m_selectionClipRect = { };
+
auto overlayHost = ([&] () -> RefPtr<HTMLElement> {
auto selectedRange = frame.selection().selection().range();
if (!selectedRange)
@@ -86,22 +91,17 @@
return;
}
- if (shouldUsePageOverlayToPaintSelection(*overlayHostRenderer)) {
- m_overlaySelectionQuads = quads;
- m_selectionBackgroundColor = overlayHostRenderer->selectionBackgroundColor();
- } else {
- m_overlaySelectionQuads.clear();
- m_selectionBackgroundColor = Color::transparentBlack;
+ if (!shouldUsePageOverlayToPaintSelection(*overlayHostRenderer)) {
+ uninstallPageOverlayIfNeeded();
+ return;
}
- m_imageOverlayBounds = overlayHostRenderer->absoluteBoundingBoxRect();
+ m_hostElementForSelection = makeWeakPtr(*overlayHost);
+ m_selectionQuads = quads;
+ m_selectionBackgroundColor = overlayHostRenderer->selectionBackgroundColor();
+ m_selectionClipRect = overlayHostRenderer->absoluteBoundingBoxRect();
-#if PLATFORM(MAC)
- updateDataDetectorHighlights(*overlayHost);
-#endif
-
- if (auto& overlay = installPageOverlayIfNeeded(); !m_overlaySelectionQuads.isEmpty())
- overlay.setNeedsDisplay();
+ installPageOverlayIfNeeded().setNeedsDisplay();
}
bool ImageOverlayController::shouldUsePageOverlayToPaintSelection(const RenderElement& renderer)
@@ -113,8 +113,15 @@
void ImageOverlayController::documentDetached(const Document& document)
{
- if (&document == m_currentOverlayDocument)
- uninstallPageOverlayIfNeeded();
+ if (m_hostElementForSelection && &document == &m_hostElementForSelection->document())
+ m_hostElementForSelection = nullptr;
+
+#if PLATFORM(MAC)
+ if (m_hostElementForDataDetectors && &document == &m_hostElementForDataDetectors->document())
+ m_hostElementForDataDetectors = nullptr;
+#endif
+
+ uninstallPageOverlayIfNeeded();
}
PageOverlay& ImageOverlayController::installPageOverlayIfNeeded()
@@ -127,12 +134,12 @@
return *m_overlay;
}
-void ImageOverlayController::uninstallPageOverlayIfNeeded()
+void ImageOverlayController::uninstallPageOverlay()
{
- m_imageOverlayBounds = { };
- m_overlaySelectionQuads.clear();
+ m_hostElementForSelection = nullptr;
+ m_selectionQuads.clear();
m_selectionBackgroundColor = Color::transparentBlack;
- m_currentOverlayDocument = nullptr;
+ m_selectionClipRect = { };
#if PLATFORM(MAC)
clearDataDetectorHighlights();
@@ -145,10 +152,23 @@
m_page->pageOverlayController().uninstallPageOverlay(*overlayToUninstall, PageOverlay::FadeMode::DoNotFade);
}
+void ImageOverlayController::uninstallPageOverlayIfNeeded()
+{
+ if (m_hostElementForSelection)
+ return;
+
+#if PLATFORM(MAC)
+ if (m_hostElementForDataDetectors)
+ return;
+#endif
+
+ uninstallPageOverlay();
+}
+
void ImageOverlayController::willMoveToPage(PageOverlay&, Page* page)
{
if (!page)
- uninstallPageOverlayIfNeeded();
+ uninstallPageOverlay();
}
void ImageOverlayController::drawRect(PageOverlay& pageOverlay, GraphicsContext& context, const IntRect& dirtyRect)
@@ -161,11 +181,11 @@
GraphicsContextStateSaver stateSaver(context);
context.clearRect(dirtyRect);
- if (m_overlaySelectionQuads.isEmpty())
+ if (m_selectionQuads.isEmpty())
return;
Path coalescedSelectionPath;
- for (auto& quad : m_overlaySelectionQuads) {
+ for (auto& quad : m_selectionQuads) {
coalescedSelectionPath.moveTo(quad.p1());
coalescedSelectionPath.addLineTo(quad.p2());
coalescedSelectionPath.addLineTo(quad.p3());
@@ -175,7 +195,7 @@
}
context.setFillColor(m_selectionBackgroundColor);
- context.clip(m_imageOverlayBounds);
+ context.clip(m_selectionClipRect);
context.fillPath(coalescedSelectionPath);
}
@@ -186,6 +206,10 @@
return false;
}
+void ImageOverlayController::elementUnderMouseDidChange(Frame&, Element*)
+{
+}
+
#endif // !PLATFORM(MAC)
} // namespace WebCore
Modified: trunk/Source/WebCore/page/ImageOverlayController.h (278330 => 278331)
--- trunk/Source/WebCore/page/ImageOverlayController.h 2021-06-01 22:36:13 UTC (rev 278330)
+++ trunk/Source/WebCore/page/ImageOverlayController.h 2021-06-01 22:36:56 UTC (rev 278331)
@@ -38,6 +38,7 @@
namespace WebCore {
class Document;
+class Element;
class Frame;
class GraphicsContext;
class HTMLElement;
@@ -57,6 +58,8 @@
explicit ImageOverlayController(Page&);
void selectionQuadsDidChange(Frame&, const Vector<FloatQuad>&);
+ void elementUnderMouseDidChange(Frame&, Element*);
+
void documentDetached(const Document&);
private:
@@ -69,6 +72,7 @@
PageOverlay& installPageOverlayIfNeeded();
void uninstallPageOverlayIfNeeded();
+ void uninstallPageOverlay();
#if PLATFORM(MAC)
void updateDataDetectorHighlights(const HTMLElement&);
@@ -78,13 +82,14 @@
DataDetectorHighlight* activeHighlight() const final { return m_activeDataDetectorHighlight.get(); }
#endif
+ void platformUpdateElementUnderMouse(Frame&, Element* elementUnderMouse);
bool platformHandleMouseEvent(const PlatformMouseEvent&);
WeakPtr<Page> m_page;
RefPtr<PageOverlay> m_overlay;
- WeakPtr<Document> m_currentOverlayDocument;
- Vector<FloatQuad> m_overlaySelectionQuads;
- LayoutRect m_imageOverlayBounds;
+ WeakPtr<HTMLElement> m_hostElementForSelection;
+ Vector<FloatQuad> m_selectionQuads;
+ LayoutRect m_selectionClipRect;
Color m_selectionBackgroundColor { Color::transparentBlack };
#if PLATFORM(MAC)
@@ -91,6 +96,7 @@
using ContainerAndHighlight = std::pair<WeakPtr<HTMLElement>, Ref<DataDetectorHighlight>>;
Vector<ContainerAndHighlight> m_dataDetectorContainersAndHighlights;
RefPtr<DataDetectorHighlight> m_activeDataDetectorHighlight;
+ WeakPtr<HTMLElement> m_hostElementForDataDetectors;
#endif
};
Modified: trunk/Source/WebCore/page/Page.h (278330 => 278331)
--- trunk/Source/WebCore/page/Page.h 2021-06-01 22:36:13 UTC (rev 278330)
+++ trunk/Source/WebCore/page/Page.h 2021-06-01 22:36:56 UTC (rev 278331)
@@ -508,6 +508,7 @@
ServicesOverlayController& servicesOverlayController() { return *m_servicesOverlayController; }
#endif
ImageOverlayController& imageOverlayController() { return *m_imageOverlayController; }
+ ImageOverlayController* imageOverlayControllerIfExists() { return m_imageOverlayController.get(); }
#if ENABLE(WHEEL_EVENT_LATCHING)
ScrollLatchingController& scrollLatchingController();
Modified: trunk/Source/WebCore/page/mac/ImageOverlayControllerMac.mm (278330 => 278331)
--- trunk/Source/WebCore/page/mac/ImageOverlayControllerMac.mm 2021-06-01 22:36:13 UTC (rev 278330)
+++ trunk/Source/WebCore/page/mac/ImageOverlayControllerMac.mm 2021-06-01 22:36:56 UTC (rev 278331)
@@ -176,10 +176,56 @@
void ImageOverlayController::clearDataDetectorHighlights()
{
+ m_hostElementForDataDetectors = nullptr;
m_dataDetectorContainersAndHighlights.clear();
m_activeDataDetectorHighlight = nullptr;
}
+void ImageOverlayController::elementUnderMouseDidChange(Frame& frame, Element* elementUnderMouse)
+{
+ if (m_activeDataDetectorHighlight)
+ return;
+
+ if (!elementUnderMouse && m_hostElementForDataDetectors && frame.document() != &m_hostElementForDataDetectors->document())
+ return;
+
+ if (!elementUnderMouse || !HTMLElement::isInsideImageOverlay(*elementUnderMouse)) {
+ m_hostElementForDataDetectors = nullptr;
+ uninstallPageOverlayIfNeeded();
+ return;
+ }
+
+ auto shadowHost = elementUnderMouse->shadowHost();
+ if (!is<HTMLElement>(shadowHost)) {
+ ASSERT_NOT_REACHED();
+ m_hostElementForDataDetectors = nullptr;
+ uninstallPageOverlayIfNeeded();
+ return;
+ }
+
+ auto imageOverlayHost = makeRef(downcast<HTMLElement>(*shadowHost));
+ if (!imageOverlayHost->hasImageOverlay()) {
+ ASSERT_NOT_REACHED();
+ m_hostElementForDataDetectors = nullptr;
+ uninstallPageOverlayIfNeeded();
+ return;
+ }
+
+ if (m_hostElementForDataDetectors == imageOverlayHost.ptr())
+ return;
+
+ updateDataDetectorHighlights(imageOverlayHost.get());
+
+ if (m_dataDetectorContainersAndHighlights.isEmpty()) {
+ m_hostElementForDataDetectors = nullptr;
+ uninstallPageOverlayIfNeeded();
+ return;
+ }
+
+ m_hostElementForDataDetectors = makeWeakPtr(imageOverlayHost.get());
+ installPageOverlayIfNeeded();
+}
+
} // namespace WebCore
#endif // PLATFORM(MAC)
