.
Created attachment 429763 [details] Patch
Comment on attachment 429763 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=429763&action=review > Source/WebCore/editing/cocoa/DataDetection.h:44 > +class HTMLElement; Nit: T < i > Source/WebCore/editing/cocoa/DataDetection.h:49 > +#if ENABLE(IMAGE_EXTRACTION) > +struct ImageExtractionDataDetectorInfo; > +#endif Nit: I think it's preferable to remove the `#if` here since the forward declaration is harmless even when `ENABLE(IMAGE_EXTRACTION)` is false. > Source/WebCore/page/Frame.h:104 > +#if ENABLE(DATA_DETECTION) > +class DataDetectionResultsStorage; > +#endif Nit: ditto > Source/WebCore/page/cocoa/DataDetectionResultsStorage.h:43 > + DataDetectionResultsStorage() = default; > + ~DataDetectionResultsStorage() = default; Nit: I think the compiler will generate a default constructor and destructor without you having to declare them. Note that you also get a default copy constructor. Do you want to `WTF_MAKE_NONCOPYABLE` this?
Comment on attachment 429763 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=429763&action=review r=me as well :) > Source/WebCore/page/Frame.cpp:112 > +#include "DataDetectionResultsStorage.h" I'd remove the `#if` here since everything in the file has the same `#if`. > Source/WebKit/WebProcess/WebPage/WebPage.cpp:273 > +#include <WebCore/DataDetectionResultsStorage.h> ditto > Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm:156 > +#if ENABLE(DATA_DETECTION) ditto
Comment on attachment 429763 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=429763&action=review >> Source/WebCore/editing/cocoa/DataDetection.h:44 >> +class HTMLElement; > > Nit: T < i Fixed! >> Source/WebCore/editing/cocoa/DataDetection.h:49 >> +#endif > > Nit: I think it's preferable to remove the `#if` here since the forward declaration is harmless even when `ENABLE(IMAGE_EXTRACTION)` is false. Sounds good! Removed the guard. >> Source/WebCore/page/Frame.cpp:112 >> +#include "DataDetectionResultsStorage.h" > > I'd remove the `#if` here since everything in the file has the same `#if`. 👍🏻 >> Source/WebCore/page/Frame.h:104 >> +#endif > > Nit: ditto Fixed! >> Source/WebCore/page/cocoa/DataDetectionResultsStorage.h:43 >> + ~DataDetectionResultsStorage() = default; > > Nit: I think the compiler will generate a default constructor and destructor without you having to declare them. > > Note that you also get a default copy constructor. Do you want to `WTF_MAKE_NONCOPYABLE` this? Good catch! Removed the ctor/dtor definitions here, and added `WTF_MAKE_NONCOPYABLE(DataDetectionResultsStorage)` above. >> Source/WebKit/WebProcess/WebPage/WebPage.cpp:273 >> +#include <WebCore/DataDetectionResultsStorage.h> > > ditto I'm not actually sure this will work, since DataDetectionResultsStorage.h (which is in a cocoa directory in WebCore) would need to be added to the Headers.cmake list to be exported. I'll see what the bots think! >> Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm:156 >> +#if ENABLE(DATA_DETECTION) > > ditto Sounds good! Removed the #if (this is Cocoa-specific code so this should be safe to import).
Created attachment 429906 [details] For EWS
Created attachment 429911 [details] Fix GTK/WPE build
Committed r278175 (238218@main): <https://commits.webkit.org/238218@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 429911 [details].
<rdar://problem/78584057>