[Webkit-unassigned] [Bug 188477] [macOS] Color wells should appear pressed when presenting a color picker

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=188477

--- Comment #3 from Aditya Keerthi <akeerthi at apple.com> ---
(In reply to Tim Horton from comment #2)
> Comment on attachment 346921 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=346921&action=review
> 
> > Source/WebCore/platform/mac/ThemeMac.mm:714
> > +    NSButtonCell *buttonCell = button(ColorWellPart, controlStates, IntSize(zoomedRect.size()), zoomFactor);
> 
> This button is kept statically, right? Who unsets highlighted?

Each time a button cell is created, setUpButtonCell is called. This in turn calls updateStates, which sets the correct highlighted value given the state. Both of those methods are in ThemeMac. Now that I think about it, it might make sense to move the statement that sets the highlight from paintColorWell, to updateStates.

> 
> > Source/WebKit/UIProcess/mac/WebColorPickerMac.mm:98
> > +    if (m_colorPickerUI) {
> 
> Why doesn't this just call endPicker?

There are two ways this destructor can be called. One is by calling endPicker() which
eventually sets m_colorPicker to nullptr in WebPageProxy. If we had endPicker() in the
destructor we would be calling endPicker() in an infinite loop.

The other way for the destructor to be called is for a new color picker to be created while the previous one is still active. For example, when clicking on another color input while one is already active. In this case, endPicker() may not be called before the new color picker is created - which is why we invalidate m_colorPickerUI.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-unassigned/attachments/20180810/d678ef4e/attachment.html>