Bug 138731 – Move DataDetectors scanning code to WebCore

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Tim Horton]

Move DataDetectors scanning code to WebCore

[Tim Horton]

Created attachment 241566 [details]
Patch

[Tim Horton]

This would be a prime candidate (one of many) for some kind of small Mac-only "WebKitKit" in between WebCore and WebKit{2}...

Comment on attachment 241566 [details]
Patch

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

> Source/WebCore/ChangeLog:22
> +        * platform/mac/DataDetection.h: Copied from Source/WebCore/editing/DictionaryLookup.h.
> +        * platform/mac/DataDetection.mm: Added.
> +        (WebCore::DataDetection::detectItemAroundHitTestResult):
> +        Moved from WebKit2.

platform/ is not the right place for this code, which depends on non-platform WebCore concepts such as the DOM and the render tree.

[Anders Carlsson]

(In reply to comment #3)
> Comment on attachment 241566 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=241566&action=review
> 
> > Source/WebCore/ChangeLog:22
> > +        * platform/mac/DataDetection.h: Copied from Source/WebCore/editing/DictionaryLookup.h.
> > +        * platform/mac/DataDetection.mm: Added.
> > +        (WebCore::DataDetection::detectItemAroundHitTestResult):
> > +        Moved from WebKit2.
> 
> platform/ is not the right place for this code, which depends on
> non-platform WebCore concepts such as the DOM and the render tree.

Agreed, maybe we can move it to page/mac/?

[Simon Fraser (smfr)]

Comment on attachment 241566 [details]
Patch

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

> Source/WebKit2/ChangeLog:9
> +        Move DataDetectors scanning code to WebCore, so that both WebKits can use it.

Is this a good long-term direction? Why not have WK2 just link with WK1?

[Anders Carlsson]

(In reply to comment #5)
> Comment on attachment 241566 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=241566&action=review
> 
> > Source/WebKit2/ChangeLog:9
> > +        Move DataDetectors scanning code to WebCore, so that both WebKits can use it.
> 
> Is this a good long-term direction? Why not have WK2 just link with WK1?

It already does. 😑

[Tim Horton]

Oh yes, of course, totally wrong place.

(In reply to comment #5)
> Comment on attachment 241566 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=241566&action=review
> 
> > Source/WebKit2/ChangeLog:9
> > +        Move DataDetectors scanning code to WebCore, so that both WebKits can use it.
> 
> Is this a good long-term direction? Why not have WK2 just link with WK1?

Ideal long term answer, in my book, is https://bugs.webkit.org/show_bug.cgi?id=138731#c, not hiding things in WebKit legacy.

I don’t think there’s anuthing wrong, conceptually or practically, with this code being in WebCore.

[Geoffrey Garen]

> Agreed, maybe we can move it to page/mac/?

Or editing, perhaps?

Editing already handles related "scan the text and possibly display something" tasks like dictionary lookup, alternative text presentation, linkification, and so on.

[Tim Horton]

Created attachment 241621 [details]
Patch

[Tim Horton]

http://trac.webkit.org/changeset/176137

[David Kilzer (:ddkilzer)]

(In reply to comment #11)
> http://trac.webkit.org/changeset/176137

iOS build fix committed as r176156: <http://trac.webkit.org/changeset/176156>