Summary: | Refactor ServiceOverlayController in preparation for fading between highlights | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Tim Horton <thorton> | ||||||
Component: | WebKit2 | Assignee: | Tim Horton <thorton> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | beidson, enrica | ||||||
Priority: | P2 | Keywords: | InRadar | ||||||
Version: | 528+ (Nightly build) | ||||||||
Hardware: | Unspecified | ||||||||
OS: | Unspecified | ||||||||
Attachments: |
|
Description
Tim Horton
2014-08-09 23:34:45 PDT
Created attachment 236340 [details]
patch
The primary motivation for this large change is the Highlight object; for fade, we need to be able to keep around/paint Highlights that might no longer be potentials (and certainly won't be 'active'), so with a refcounted Highlight object we'll be able to keep them around separately and refer to them consistently in various places. Hackability/cleanup was a secondary motivation. Comment on attachment 236340 [details]
patch
not quite done yet
Comment on attachment 236340 [details]
patch
huh, the thing I found is not a regression from this patch like I assumed it was. re-r?ing
Created attachment 236342 [details]
patch
Comment on attachment 236342 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=236342&action=review > Source/WebKit2/WebProcess/WebPage/ServicesOverlayController.h:66 > + enum class Type { TelephoneNumber, Selection }; Even if only two thingies, I like seeing this on multiple lines. enum class Type { Thingy1, Thingy2, }; > Source/WebKit2/WebProcess/WebPage/mac/ServicesOverlayController.mm:242 > + bool hovered = DDHighlightPointIsOnHighlight(highlight.highlight(), (CGPoint)m_mousePosition, &onButton); highlight.highlight() reads weirdly to me. Could the DDHighlightRef accessor be "ddHighlight()" or "ddHighlightRef()"? It's not too mysterious here because it's being fed into a DD API, but it would be mysterious without that context. Comment on attachment 236342 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=236342&action=review >> Source/WebKit2/WebProcess/WebPage/ServicesOverlayController.h:66 >> + enum class Type { TelephoneNumber, Selection }; > > Even if only two thingies, I like seeing this on multiple lines. > > enum class Type { > Thingy1, > Thingy2, > }; I like them on one line. |