Summary: | Move image overlay code out of HTMLElement and into a separate helper file | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Wenson Hsieh <wenson_hsieh> | ||||||||
Component: | Platform | Assignee: | Wenson Hsieh <wenson_hsieh> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | akeerthi, annulen, calvaris, cdumez, changseok, eric.carlson, esprehn+autocc, ews-watchlist, glenn, gyuyoung.kim, hi, jer.noble, kangil.han, koivisto, kondapallykalyan, megan_gardner, mifenton, pdr, philipj, ryuan.choi, sergio, thorton, webkit-bug-importer | ||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||
Version: | WebKit Nightly Build | ||||||||||
Hardware: | Unspecified | ||||||||||
OS: | Unspecified | ||||||||||
Attachments: |
|
Description
Wenson Hsieh
2021-11-10 17:12:18 PST
Comment hidden (obsolete)
The existing helpers on HTMLElement should belong in a separate header/compilation unit, instead of drifting around in HTMLElement.(h|cpp) Created attachment 443953 [details]
For EWS
Created attachment 443958 [details]
Fix Windows build
Comment on attachment 443958 [details] Fix Windows build View in context: https://bugs.webkit.org/attachment.cgi?id=443958&action=review > Source/WebCore/ChangeLog:10 > + Move code for updating, querying, and removing image overlays out of HTMLElement.cpp and HTMLElement.h and into > + separate helper files instead (ImageOverlay.h and ImageOverlay.cpp). Future patches in this area will extend the > + functionality of these overlays, so this will help avoid code bloat inside HTMLElement when doing so. Nice! > Source/WebCore/dom/ImageOverlay.cpp:168 > +void updateWithTextRecognitionResult(HTMLElement& element, const TextRecognitionResult& result, CacheTextRecognitionResults cacheTextRecognitionResults) Would be nice to chop up this mega-function into smaller pieces at some point. Comment on attachment 443958 [details] Fix Windows build View in context: https://bugs.webkit.org/attachment.cgi?id=443958&action=review > Source/WebCore/dom/ImageOverlay.h:50 > +WEBCORE_EXPORT bool hasImageOverlay(const HTMLElement&); > +WEBCORE_EXPORT bool isImageOverlayDataDetectorResult(const HTMLElement&); > +WEBCORE_EXPORT bool isInsideImageOverlay(const SimpleRange&); > +WEBCORE_EXPORT bool isInsideImageOverlay(const Node&); > +WEBCORE_EXPORT bool isImageOverlayText(const Node&); > +WEBCORE_EXPORT bool isImageOverlayText(const Node*); > +void removeImageOverlaySoonIfNeeded(HTMLElement&); > +IntRect containerRectForImageOverlay(HTMLElement&); > + > +#if ENABLE(IMAGE_ANALYSIS) > +enum class CacheTextRecognitionResults : bool { No, Yes }; > +WEBCORE_EXPORT void updateWithTextRecognitionResult(HTMLElement&, const TextRecognitionResult&, CacheTextRecognitionResults = CacheTextRecognitionResults::Yes); > +#endif You could consider wrapping these functions into a namespace or a class (say "ImageOverlay"). Comment on attachment 443958 [details] Fix Windows build View in context: https://bugs.webkit.org/attachment.cgi?id=443958&action=review Thanks for the review! >> Source/WebCore/dom/ImageOverlay.cpp:168 >> +void updateWithTextRecognitionResult(HTMLElement& element, const TextRecognitionResult& result, CacheTextRecognitionResults cacheTextRecognitionResults) > > Would be nice to chop up this mega-function into smaller pieces at some point. Good point! I'll do this refactoring as part of a future patch that'll augment this code. >> Source/WebCore/dom/ImageOverlay.h:50 >> +#endif > > You could consider wrapping these functions into a namespace or a class (say "ImageOverlay"). Makes sense — I'll wrap these helpers in a `namespace ImageOverlay` in this patch. Created attachment 443975 [details]
Add namespace, fix CMake builds
Tools/Scripts/svn-apply failed to apply attachment 443975 [details] to trunk.
Please resolve the conflicts and upload a new patch.
Committed r285655 (244153@main): <https://commits.webkit.org/244153@main> |