Skip to content

Commit

Permalink
[UnifiedPDF] Text annotation HTML inputs do not get repositioned or s…
Browse files Browse the repository at this point in the history
…caled when zooming

https://bugs.webkit.org/show_bug.cgi?id=268360
rdar://121907350

Reviewed by Tim Horton.

UnifiedPDFPlugin was incorrectly returning the annotation bounds in
document space when the PDFPluginAnnotation was expecting the bounds in
plugin view space. Fix this by providing the bounds in the correct coordinate
space so that it can update its geomtetry correctly. This requires
that PDFPluginAnnotation stops subtracting out the scroll positions
since this is part of the document to plugin space translation.

The size of the annotation should be determined with both the document
fitting sale and the zoom scale.

Also rename boundsForannotation to pluginBoundsForAnnotation to make it
clear what coordinate space this function is returning geometry in.

* Source/WebKit/WebProcess/Plugins/PDF/PDFPlugin.h:
* Source/WebKit/WebProcess/Plugins/PDF/PDFPlugin.mm:
(WebKit::PDFPlugin::pluginBoundsForAnnotation const):
(WebKit::PDFPlugin::boundsForAnnotation const): Deleted.
* Source/WebKit/WebProcess/Plugins/PDF/PDFPluginAnnotation.mm:
(WebKit::PDFPluginAnnotation::updateGeometry):
* Source/WebKit/WebProcess/Plugins/PDF/PDFPluginBase.h:
* Source/WebKit/WebProcess/Plugins/PDF/UnifiedPDF/UnifiedPDFPlugin.h:
* Source/WebKit/WebProcess/Plugins/PDF/UnifiedPDF/UnifiedPDFPlugin.mm:
(WebKit::UnifiedPDFPlugin::convertFromDocumentToPlugin const):
(WebKit::UnifiedPDFPlugin::pluginBoundsForAnnotation const):
(WebKit::UnifiedPDFPlugin::boundsForAnnotation const): Deleted.

Canonical link: https://commits.webkit.org/274038@main
  • Loading branch information
sammygill committed Feb 3, 2024
1 parent 7223f04 commit 9e6c27d
Show file tree
Hide file tree
Showing 6 changed files with 25 additions and 13 deletions.
2 changes: 1 addition & 1 deletion Source/WebKit/WebProcess/Plugins/PDF/PDFPlugin.h
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ class PDFPlugin final : public PDFPluginBase {
void performWebSearch(NSString *);
void performSpotlightSearch(NSString *);

CGRect boundsForAnnotation(RetainPtr<PDFAnnotation>&) const final;
CGRect pluginBoundsForAnnotation(RetainPtr<PDFAnnotation>&) const final;
void focusNextAnnotation() final;
void focusPreviousAnnotation() final;

Expand Down
5 changes: 3 additions & 2 deletions Source/WebKit/WebProcess/Plugins/PDF/PDFPlugin.mm
Original file line number Diff line number Diff line change
Expand Up @@ -1418,11 +1418,12 @@ static bool getEventTypeFromWebEvent(const WebEvent& event, NSEventType& eventTy
return true;
}

CGRect PDFPlugin::boundsForAnnotation(RetainPtr<PDFAnnotation>& annotation) const
CGRect PDFPlugin::pluginBoundsForAnnotation(RetainPtr<PDFAnnotation>& annotation) const
{
auto documentSize = contentSizeRespectingZoom();
auto annotationBounds = [m_pdfLayerController boundsForAnnotation:annotation.get()];
annotationBounds.origin.y = documentSize.height - annotationBounds.origin.y - annotationBounds.size.height;
annotationBounds.origin.y = documentSize.height - annotationBounds.origin.y - annotationBounds.size.height - m_scrollOffset.height();
annotationBounds.origin.x -= m_scrollOffset.width();
return annotationBounds;
}

Expand Down
7 changes: 3 additions & 4 deletions Source/WebKit/WebProcess/Plugins/PDF/PDFPluginAnnotation.mm
Original file line number Diff line number Diff line change
Expand Up @@ -105,14 +105,13 @@

void PDFPluginAnnotation::updateGeometry()
{
NSRect annotationRect = NSRectFromCGRect(m_plugin->boundsForAnnotation(m_annotation));
NSRect annotationRect = NSRectFromCGRect(m_plugin->pluginBoundsForAnnotation(m_annotation));

StyledElement* styledElement = static_cast<StyledElement*>(element());
styledElement->setInlineStyleProperty(CSSPropertyWidth, annotationRect.size.width, CSSUnitType::CSS_PX);
styledElement->setInlineStyleProperty(CSSPropertyHeight, annotationRect.size.height, CSSUnitType::CSS_PX);
IntPoint scrollPosition(m_plugin->scrollPosition());
styledElement->setInlineStyleProperty(CSSPropertyLeft, annotationRect.origin.x - scrollPosition.x(), CSSUnitType::CSS_PX);
styledElement->setInlineStyleProperty(CSSPropertyTop, annotationRect.origin.y - scrollPosition.y(), CSSUnitType::CSS_PX);
styledElement->setInlineStyleProperty(CSSPropertyLeft, annotationRect.origin.x, CSSUnitType::CSS_PX);
styledElement->setInlineStyleProperty(CSSPropertyTop, annotationRect.origin.y, CSSUnitType::CSS_PX);
}

bool PDFPluginAnnotation::handleEvent(Event& event)
Expand Down
2 changes: 1 addition & 1 deletion Source/WebKit/WebProcess/Plugins/PDF/PDFPluginBase.h
Original file line number Diff line number Diff line change
Expand Up @@ -192,7 +192,7 @@ class PDFPluginBase : public ThreadSafeRefCountedAndCanMakeThreadSafeWeakPtr<PDF
virtual void setActiveAnnotation(RetainPtr<PDFAnnotation>&&) = 0;
void didMutatePDFDocument() { m_pdfDocumentWasMutated = true; }

virtual CGRect boundsForAnnotation(RetainPtr<PDFAnnotation>&) const = 0;
virtual CGRect pluginBoundsForAnnotation(RetainPtr<PDFAnnotation>&) const = 0;
virtual void focusNextAnnotation() = 0;
virtual void focusPreviousAnnotation() = 0;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ class UnifiedPDFPlugin final : public PDFPluginBase, public WebCore::GraphicsLay
};
using PDFElementTypes = OptionSet<PDFElementType>;

CGRect boundsForAnnotation(RetainPtr<PDFAnnotation>&) const final;
CGRect pluginBoundsForAnnotation(RetainPtr<PDFAnnotation>&) const final;
void setActiveAnnotation(RetainPtr<PDFAnnotation>&&) final;
void startAnnotationTracking(RetainPtr<PDFAnnotation>&&);
void finishAnnotationTracking();
Expand Down Expand Up @@ -235,6 +235,7 @@ class UnifiedPDFPlugin final : public PDFPluginBase, public WebCore::GraphicsLay
RefPtr<WebCore::GraphicsLayer> createGraphicsLayer(const String& name, WebCore::GraphicsLayer::Type);

WebCore::IntPoint convertFromPluginToDocument(const WebCore::IntPoint&) const;
WebCore::IntPoint convertFromDocumentToPlugin(const WebCore::IntPoint&) const;
std::optional<PDFDocumentLayout::PageIndex> pageIndexForDocumentPoint(const WebCore::IntPoint&) const;
RetainPtr<PDFAnnotation> annotationForRootViewPoint(const WebCore::IntPoint&) const;
WebCore::IntPoint convertFromDocumentToPage(const WebCore::IntPoint&, PDFDocumentLayout::PageIndex) const;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -822,6 +822,18 @@
return roundedIntPoint(transformedPoint);
}

WebCore::IntPoint UnifiedPDFPlugin::convertFromDocumentToPlugin(const WebCore::IntPoint& point) const
{
auto transformedPoint = FloatPoint { point };

transformedPoint.scale(m_documentLayout.scale());
transformedPoint.move(sidePaddingWidth(), 0);
transformedPoint.scale(m_scaleFactor);
transformedPoint.moveBy(-FloatPoint(m_scrollOffset));

return roundedIntPoint(transformedPoint);
}

std::optional<PDFDocumentLayout::PageIndex> UnifiedPDFPlugin::pageIndexForDocumentPoint(const WebCore::IntPoint& point) const
{
for (PDFDocumentLayout::PageIndex index = 0; index < m_documentLayout.pageCount(); ++index) {
Expand Down Expand Up @@ -1318,7 +1330,7 @@ static IntRect computeMarqueeSelectionRect(const WebCore::IntPoint& point1, cons

#endif // ENABLE(PDF_HUD)

CGRect UnifiedPDFPlugin::boundsForAnnotation(RetainPtr<PDFAnnotation>& annotation) const
CGRect UnifiedPDFPlugin::pluginBoundsForAnnotation(RetainPtr<PDFAnnotation>& annotation) const
{
auto pageSpaceBounds = IntRect([annotation.get() bounds]);
if (auto pageIndex = m_documentLayout.indexForPage([annotation.get() page])) {
Expand All @@ -1328,9 +1340,8 @@ static IntRect computeMarqueeSelectionRect(const WebCore::IntPoint& point1, cons
// in the Y axis so we need to perform a flip
documentSpacePoint.setY(documentSpacePoint.y() - pageSpaceBounds.height());

documentSpacePoint.scale(m_documentLayout.scale());
pageSpaceBounds.scale(m_documentLayout.scale());
return { documentSpacePoint, pageSpaceBounds.size() };
pageSpaceBounds.scale(m_documentLayout.scale() * m_scaleFactor);
return { convertFromDocumentToPlugin(documentSpacePoint), pageSpaceBounds.size() };
}
ASSERT_NOT_REACHED();
return pageSpaceBounds;
Expand Down

0 comments on commit 9e6c27d

Please sign in to comment.