Bug 13303 - REGRESSION: controls in a background Safari window maintain active appearance if the address bar has focus
Summary: REGRESSION: controls in a background Safari window maintain active appearance...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Forms (show other bugs)
Version: 523.x (Safari 3)
Hardware: Macintosh OS X 10.4
: P1 Normal
Assignee: Darin Adler
URL:
Keywords: InRadar, Regression
Depends on:
Blocks:
 
Reported: 2007-04-07 15:48 PDT by mitz
Modified: 2007-04-16 09:00 PDT (History)
1 user (show)

See Also:


Attachments
patch that updates the tint based on the appropriate AppKit method (20.17 KB, patch)
2007-04-15 19:52 PDT, Darin Adler
sullivan: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description mitz 2007-04-07 15:48:29 PDT
Summary:
Mac-themed popup buttons, radio buttons, checkboxes and sliders maintain their "active" appearance (the dark color behind the arrows) in a background Safari window if the window is made background while the address bar has focus.

Steps to reproduce:
1) Go to a URL with a popup, e.g. <data:text/html,%3Cselect%3E%3Coption%3Epick%20me!%3C/option%3E%3C/select%3E>.
2) Click in Safari's address bar.
3) Click on the desktop to send the window to the background.

Expected results:
In step 3, the popup button should lose its active appearance (the dark background behind the arrows).

Actual results:
The popup button maintains active appearance.

Notes:
Those widgets do become inactive if the WebView is first responder when sending the window to the background.

Regression:
This is a regression from shipping WebKit.
Comment 1 mitz 2007-04-07 15:54:51 PDT
Forcing a repaint corrects the widgets' appearance. Using Quartz Debug you can see that if the WebView is first responder when sending the window to the background, the widgets repaint, but if the address bar is first responder, they don't.

(To force a repaint while the window is in the background, have another tab open in the window and command-click that tab to switch to it while keeping the window in the background).
Comment 2 Darin Adler 2007-04-10 09:24:19 PDT
The problem here is that repainting the widgets when the control tint changes is done in Frame::setIsActive. But the rule for when the widgets have active vs. inactive tint is not "is active".

Should not be too difficult to fix. Will require changes to Frame and to WebHTMLView as well.
Comment 3 Darin Adler 2007-04-11 01:05:41 PDT
<rdar://problem/5126341>
Comment 4 Darin Adler 2007-04-15 19:52:35 PDT
Created attachment 14046 [details]
patch that updates the tint based on the appropriate AppKit method
Comment 5 John Sullivan 2007-04-15 21:27:41 PDT
Comment on attachment 14046 [details]
patch that updates the tint based on the appropriate AppKit method

I think the names "setSecureKeyboardEntry()" and "setNeedsSecureKeyboardEntry()" are confusingly similar.

Also, you probably realize that you could learn of changes to the window's key state by listening to NSWindowDidBecomeKey/NSWindowDidResignKey rather than using _windowChangedKeyState. I presume you chose _windowChangedKeystate for simplicity/performance. Maybe worth a comment.

r=me
Comment 6 mitz 2007-04-15 22:41:44 PDT
Comment on attachment 14046 [details]
patch that updates the tint based on the appropriate AppKit method

Typo:
+    // We do a "fake" paintm, and when the theme gets a paint call, it can then do an invalidate.
Comment 7 Darin Adler 2007-04-16 08:32:21 PDT
(In reply to comment #5)
> I think the names "setSecureKeyboardEntry()" and
> "setNeedsSecureKeyboardEntry()" are confusingly similar.

OK. Maybe I can think of better names.

> Also, you probably realize that you could learn of changes to the window's key
> state by listening to NSWindowDidBecomeKey/NSWindowDidResignKey rather than
> using _windowChangedKeyState. I presume you chose _windowChangedKeystate for
> simplicity/performance. Maybe worth a comment.

Actually, _windowChangedKeystate sounds like it's the same thing as NSWindowDidBecomeKey, but it's actually *quite* a bit different. It does just the right thing for control appearance which is way more than just "is this particular window key". I would like to add a comment about that.
Comment 8 Darin Adler 2007-04-16 09:00:01 PDT
Sending        WebCore/ChangeLog
Sending        WebCore/WebCore.exp
Sending        WebCore/html/HTMLInputElement.cpp
Sending        WebCore/page/Frame.cpp
Sending        WebCore/page/Frame.h
Sending        WebCore/page/FramePrivate.h
Sending        WebCore/page/FrameView.cpp
Sending        WebCore/page/FrameView.h
Sending        WebCore/page/mac/FrameMac.mm
Sending        WebCore/rendering/RenderTheme.cpp
Sending        WebCore/rendering/RenderThemeMac.mm
Sending        WebKit/ChangeLog
Sending        WebKit/WebView/WebHTMLView.mm
Transmitting file data .............
Committed revision 20901.