Bug 239460

Created attachment 457818 [details]
Patch

Created attachment 457823 [details]
Patch

Created attachment 457828 [details]
Patch

Created attachment 457834 [details]
Patch

Created attachment 457839 [details]
Patch

Created attachment 457842 [details]
Patch

Created attachment 457848 [details]
Patch

Created attachment 457849 [details]
Patch

Created attachment 457850 [details]
Patch

Created attachment 457854 [details]
Patch

Comment on attachment 457854 [details]
Patch

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

> Source/WebCore/dom/Element.h:318
> +    ShadowRoot* shadowRoot() const;

If we add the inline keyword to the declaration of shadowRoot (like replace WEBCORE_EXPORT with inline) then, I seem to recall, we get compiler errors rather than linker errors when we call it without remembering to include ElementRareData.h. So I suggest including that keyword. We should use that technique more! I think Jer Noble is the one who pointed this out to me.

> Source/WebCore/editing/cocoa/DataDetection.mm:694
> +std::optional<std::pair<Ref<HTMLElement>, IntRect>> DataDetection::findDataDetectionResultElementInImageOverlay(const FloatPoint& location, const HTMLElement& imageOverlayHost)

This seems like a really good change, but not mentioned in change log.

(In reply to Darin Adler from comment #11)
> Comment on attachment 457854 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=457854&action=review
> 
> > Source/WebCore/dom/Element.h:318
> > +    ShadowRoot* shadowRoot() const;
> 
> If we add the inline keyword to the declaration of shadowRoot (like replace
> WEBCORE_EXPORT with inline) then, I seem to recall, we get compiler errors
> rather than linker errors when we call it without remembering to include
> ElementRareData.h. So I suggest including that keyword. We should use that
> technique more! I think Jer Noble is the one who pointed this out to me.
> 
> > Source/WebCore/editing/cocoa/DataDetection.mm:694
> > +std::optional<std::pair<Ref<HTMLElement>, IntRect>> DataDetection::findDataDetectionResultElementInImageOverlay(const FloatPoint& location, const HTMLElement& imageOverlayHost)
> 
> This seems like a really good change, but not mentioned in change log.

Ok, I'll mention in the changelog. The issue is that we'd need to include ElementRareData.h from the WebKit layer otherwise. And this is currently not possible because this header is not exported.

(In reply to Darin Adler from comment #11)
> Comment on attachment 457854 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=457854&action=review
> 
> > Source/WebCore/dom/Element.h:318
> > +    ShadowRoot* shadowRoot() const;
> 
> If we add the inline keyword to the declaration of shadowRoot (like replace
> WEBCORE_EXPORT with inline) then, I seem to recall, we get compiler errors
> rather than linker errors when we call it without remembering to include
> ElementRareData.h. So I suggest including that keyword. We should use that
> technique more! I think Jer Noble is the one who pointed this out to me.

Nice trick. This is good to know, thanks. I'll adopt in this patch.

Created attachment 457913 [details]
Patch

Created attachment 457921 [details]
Patch

Committed r293042 (249781@main): <https://commits.webkit.org/249781@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 457921 [details].

<rdar://problem/91992406>