[Webkit-unassigned] [Bug 28108] Tint of checkboxes/radio buttons/etc. not correctly updated on Chromium/Mac

[bugzilla-daemon at webkit.org]

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





--- Comment #13 from Viet-Trung Luu <viettrungluu at gmail.com>  2009-08-10 15:18:53 PDT ---
(In reply to comment #9)
> Could you add a more complete explanation here including answers to the
> following questions and anything else someone looking at this code further down
> the road:
> * Why do we need this when Safari doesn't?

When we draw our controls using Cocoa, we feed them inView:nil. The code in
RenderThemeMac.mm (most similar to ours) usually feeds
inView:o->view()->frameView()->documentView(), i.e., a real NSView, from which
the control can determine the "active" state. The current code in
RenderThemeSafari.cpp checks isActive() in its determineState().

[In the latest version of the patch, I use isActive(), which does what I
previously did manually.]

> * Are controls repainted when in background?

They are repainted whenever the FocusController's setActive() is called and
this actually results in a changed state (setActive() checks). We only call
down to setActive() when necessary, for the foreground view.

> * Comment with a summary of Avi's note.
> 
> > +static void updateNSCellControlTint(NSCell* cell, RenderObject* o) {
> > +    if (o->document()->frame() && o->document()->frame()->page() &&
> > +            o->document()->frame()->page()->focusController()) {
> Could you split this whole check into a temp variable and then text that
> variable, focusControllerIsValid or somesuch.

This is gone, i.e., now inside the pre-existing RenderTheme::isActive().

> > +        FocusController* focusCtlr =
> > +                o->document()->frame()->page()->focusController();
> > +        [cell setControlTint:(focusCtlr->isActive() ?
> > +                                  [NSColor currentControlTint] :
> > +                                  NSClearControlTint)];
> Again temp variable and lose the ?:, should make this a little easier to read.

Please take a look at the current patch (it still has a ?:, but in a
hopefully-more-reasonable context). I also lined things up with the current
updateFooState()'s.

Note that since all NSCells have an "active state"/control tint, I set said
state for everything. If one is very confident, one could remove the setting of
the active state for various controls (but I prefer to play it safe).

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.