Closed Bug 54488 Opened 24 years ago Closed 16 years ago

[Mac] Non-draggable widgets in background windows should look disabled

Categories

(Core :: Widget: Cocoa, defect, P3)

All
macOS
defect

Tracking

()

VERIFIED FIXED
mozilla1.9.1b1

People

(Reporter: mpt, Assigned: mstange)

References

(Blocks 1 open bug)

Details

(Keywords: platform-parity, polish, regression, Whiteboard: [not needed for 1.9])

Attachments

(7 files, 11 obsolete files)

(deleted), image/png
Details
(deleted), patch
jaas
: review+
Details | Diff | Splinter Review
(deleted), patch
Details | Diff | Splinter Review
(deleted), image/png
Details
(deleted), image/png
Details
(deleted), image/png
Details
(deleted), patch
Details | Diff | Splinter Review
When a window is not the frontmost window of the frontmost app, all widgets in it should look disabled (as is done with native apps), to signify that simply clicking on the widget will not do anything to the widget (it will bring the window to the front instead). The exception is for any widgets which can be dragged anywhere (e.g. the proxy icon, bookmarks etc); these should be left enabled to indicate that they can be dragged even when the window is not frontmost. It *might* be worth really and truly disabling the widgets while they are in a background window, which would fix bug 6582 for free. However, depending on the way the code is architected, to do that you would probably need to keep some sort of list of which widgets were disabled and which enabled, so you could restore the correct states when the window became the frontmost again. And that might be more trouble than it was worth.
This is arguably invalid while bug 6582 exists, since the buttons currently correctly show clickability. Toolbars and toolbar buttons may also be draggable, as are scrollbars now. I'm not sure that means they should be draggable while in background, unless you could drag them to another window. ->future
Status: NEW → ASSIGNED
Target Milestone: --- → Future
Background buttons do not currently correctly display clickability, if Simon's comment in bug 6582 that `the first click in a background window simply brings the window to the front' is currently correct for Mozilla. As for dragability, I obviously wasn't precise enough ... For `any widgets which can be dragged anywhere', read `any widgets which can be dragged outside their own window'.
The click is currently used by the widget, at least on Win32.
->pinkerton, leaving future target.
Assignee: trudelle → pinkerton
Status: ASSIGNED → NEW
*** Bug 71724 has been marked as a duplicate of this bug. ***
Keywords: pp
On Mac OS X, it is OK to support click-through for some widgets if it makes sense and is non-destructive. This is documented on pp. 60-61 in the Macworld draft of the Aqua HIG. http://developer.apple.com/techpubs/macosx/SystemOverview/AquaHIGuidelines/
That behavior on Mac OS X: (1) is an extension of behavior already found in Mac OS Classic (notably in Finder and Launcher icons), (2) in its extended form is (in my humble opinion) yet another silly design decison in Aqua, (3) is *optional*, and (4) should really be in a separate bug from this one. (See also bug 51323.)
It's not just for OS X apps. Any Mac APPL can do it if "Accept Background Events" is on. Until this bug is fixed, it might be a good idea to turn it on.
Hyatt, I discussed this bug with Ben and he said you're the man. Apparently we need: 1. an array of controls for each window, hashed by id, so we can quickly toggle them on/off without having to explore the entire DOM; 2. a controller to be a required part of the binding for every window, so that relevant widgets can be enabled/disabled when the window gains/loses focus; 3. a platform-specific way of telling which controls should be disabled when the window loses focus (including interrogating them to see whether they can be dragged outside their own window).
*** Bug 76214 has been marked as a duplicate of this bug. ***
*** Bug 124736 has been marked as a duplicate of this bug. ***
*** Bug 141478 has been marked as a duplicate of this bug. ***
Blocks: 73812
Keywords: mozilla1.2, polish
Keywords: mozilla1.2mozilla1.3
OS: Mac System 8.5 → MacOS X
See also bug 38646, "mousedown on something draggable shouldn't give focus to mozilla".
*** Bug 217489 has been marked as a duplicate of this bug. ***
Should a separate report be filed for the Firefox version of this bug, or will one fix take care of both? If there's a Firefox bug report, I haven't seen it yet.
Wayne, that "XP Toolkit/Widgets" is in the "Browser" product is an accident of history. The component applies to Firefox just as much. No separate bug necessary.
Assignee: mikepinkerton → mark
Component: XP Toolkit/Widgets → Widget: Mac
Attached patch Patch (obsolete) (deleted) — Splinter Review
The only native controls we currently use are scrollbars, and it's proper to activate/deactivate scrollbar appearance along with the windows that contain them. kEventWindowActivated/kEventWindowDeactivated handlers are installed to address this, and the window's active state when the control is created is used to set the control's initial active state.
Attachment #223227 - Flags: review?(joshmoz)
Attachment #223227 - Flags: review?(joshmoz) → review+
That's a good start -- but when I reported this bug even Mozilla's scrollbars weren't native, so I wasn't referring just to non-existent native controls! XUL toolbar buttons, checkboxes, radiobuttons, option menus, and so on should similarly look unavailable when in background windows; and all those same controls (except, matching an OS X oddity, scrollbars) should actually *be* unavailable when in background windows. The exceptions are things that can be dragged from the background window, such as bookmarks and mail messages.
Attached patch Patch v2 (checked in on trunk) (deleted) — Splinter Review
This is almost the same as v1, except it uses ::HiliteControl, which I'd ordinarily avoid, to better integrate with other callers to ::HiliteControl elsewhere in widget/src/mac. The only functional difference from v1 is that this will avoid enabling a scrollbar that should be disabled because it's not scrollable - see nsNativeScrollbar::GetControlHiliteState().
Attachment #223227 - Attachment is obsolete: true
Attachment #223329 - Flags: review?(joshmoz)
(In reply to comment #19) > That's a good start -- but when I reported this bug even Mozilla's scrollbars > weren't native, so I wasn't referring just to non-existent native controls! OK, I'll just do the scrollbars for now. Non-native widgets will need some more support (and will remain a problem in cocoa widgets). Native form controls are still a problem too, because we're just slapping them up with DrawThemeButton (even in cocoa). I'll leave this bug open for possible subsequent fixes after the scrollbars.
Attachment #223329 - Flags: review?(joshmoz) → review+
Comment on attachment 223329 [details] [diff] [review] Patch v2 (checked in on trunk) a1.8.1? on me as a reminder
Attachment #223329 - Attachment description: Patch v2 → Patch v2 (checked in on trunk)
Attachment #223329 - Flags: approval-branch-1.8.1?(mark)
Depends on: 339370
Could this have caused bug 339482?
No, regression range is wrong.
Depends on: 339482
Checked in on MOZILLA_1_8_BRANCH.
Attachment #223824 - Flags: approval-branch-1.8.1+
Attachment #223329 - Flags: approval-branch-1.8.1?(mark)
Depends on: 340027
Blocks: 262389
*** Bug 338482 has been marked as a duplicate of this bug. ***
*** Bug 242678 has been marked as a duplicate of this bug. ***
Depends on: 353419
I'd be interested in working on this bug. Do you mind if I take it?
Be my guest.
bug taken. I'll work on this after I finish bug 370439.
Assignee: mark → cbarrett
Moving this to a relevant component. This is now a regression for scrollbars (from bug 370439).
Component: Widget: Mac → Widget: Cocoa
Flags: blocking1.9?
Keywords: regression
QA Contact: jrgmorrison → cocoa
Flags: blocking1.9? → blocking1.9+
Target Milestone: Future → mozilla1.9alpha6
Colin - ping..
Target Milestone: mozilla1.9alpha6 → mozilla1.9beta2
This is being moved to b2 per Mac Status meeting target milestone triage.
Assignee: cbarrett → joshmoz
Attached patch fix v0.8 (obsolete) (deleted) — Splinter Review
This is a start. It works well in most cases, but it doesn't work for the popup window case (e.g. the URL bar popup). Scrollbars always appear inactive in popups because the windows don't actually have cocoa key status. I'll have to figure out a different metric for determining inactive state. What is the total list of controls that need to change their appearance when a window becomes inactive? I'll have to make this work for them too.
Attached patch fix v0.9 (obsolete) (deleted) — Splinter Review
This includes a couple changes that fix all of my known issues about scrollbars. This makes the enabled state work for popup windows and makes the active/inactive enabled state override all other states (like nothing-to-scroll).
Attachment #277577 - Attachment is obsolete: true
Comment on attachment 277597 [details] [diff] [review] fix v0.9 I'm going to add a couple comments to explain the invalidate calls in nsChildView.mm. Roc, does this approach look OK?
Looks good. Anything that sets kThemeStateActive or kThemeTrackActive should use the kThemeStateInactive or kThemeTrackInactive constant when drawing in the background, I believe. Searching for those two constants should get you everything. BTW, should we send out a general event or have some sort of CSS property for wether or not a window is active? It seems like other UI elements that imitate or want to act like native controls would find that useful. Perhaps as a spinoff bug?
> Roc, does this approach look OK? Yeah. You might want to do some Tp profiling to ensure that frameIsInActiveWindow doesn't show up. > BTW, should we send out a general event or have some sort of CSS property for > wether or not a window is active? A CSS property for this might be interesting, but I suggest you raise it with the WG --- it's not 1.9 material anyway.
I'd guess what you'd actually add want to be adding would be a psedudoclass. And I hadn't even thought about using it in content (my use cases were all chrome). That could get really interesting.
Attached patch fix v0.9.5 (obsolete) (deleted) — Splinter Review
This makes things work for popup menus, checkboxes, and radio buttons in addition to scrollbars. There are some stylistic and minor re-factoring changes that need to make it into the final version of this patch, and also this won't work for embedding.
Attachment #277597 - Attachment is obsolete: true
DrawScale should also have this. See the Delay control in the General pane of Finder's preferences. DrawTab and DrawTabPanel as well, see the Displays pane of System Preferences. Those are just the ones I could find direct examples for. Anywhere we set kThemeStateActive or kThemeTrackActive a FooDrawInfo struct should be doing this. This is important for chrome too, not just content.
Attached patch fix v1.0 (obsolete) (deleted) — Splinter Review
There are two issues that this patch points out but doesn't cause. They should be filed and fixed separately. 1. Inactive native form controls can appear washed out (compare against Safari). This is more apparent with inactive controls I think because they are more transparent. 2. This patch works in Camino, but Camino doesn't always inform its embedding views that the window has lost key status. Thus, occasionally you'll see controls looking active that should be look inactive.
Attachment #277629 - Attachment is obsolete: true
Attachment #277940 - Flags: review?(cbarrett)
Comment on attachment 277940 [details] [diff] [review] fix v1.0 There are two places I noticed that we have a comment about HIThemeSetFill not being available on 10.3 Should take care of that eventually (follow up bug?) I dug in a little on the "washed out" look, and it's a limitation in Carbon -- check out TextWrangler, it does a similar sort of behavior. I otool'd and nm'd pretty thoroughly -- Cocoa uses [cell setControlTint:NSClearControlTint] to do its drawing, and I found no analog in Carbon. If we want the "right" behavior, we'll need to use NSCell to draw certain elements -- checkboxes and radio buttons are the ones that will probably be the most noticable. Luckily, they are the easiest to draw with NSCell for us since they can really only be drawn at one size. But that's all follow up bug material :) Other than Carbon being dumb, the patch looks good :)
Attachment #277940 - Flags: review?(cbarrett) → review+
(In reply to comment #44) > I dug in a little on the "washed out" look, and it's a limitation in Carbon -- > check out TextWrangler, it does a similar sort of behavior. I otool'd and nm'd > pretty thoroughly -- Cocoa uses [cell setControlTint:NSClearControlTint] to do > its drawing, and I found no analog in Carbon. I should add that HITheme doesn't use or respect the "control tint" mechanism. _NSSetDefaultControlTint(NSClearControlTint) works fine in a test Cocoa app that just draws a checkbox, but doesn't affect HITheme drawing at all. Sad.
Attachment #277940 - Flags: superreview?(roc)
What about performance --- do you have any idea what this does to Tp?
Since the invalidates are only called on activate/deactivate, I think the Tp impact is minimal. Just the extra code running to determine active status of the window.
Yeah, but that code runs every time we paint a form control, which is quite often. Oh well, just check it in and watch Tp carefully I guess.
Attachment #277940 - Flags: superreview?(roc) → superreview+
Status: NEW → RESOLVED
Closed: 17 years ago
Resolution: --- → FIXED
landed on trunk
I backed this out because I noticed that windows can get stuck in a state where they don't redraw their controls and they always appear inactive.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Blocks: 394092
Target Milestone: mozilla1.9 M9 → ---
Targeting M9 as this bug will block beta.
Target Milestone: --- → mozilla1.9 M9
Moving this to M10 due for resource reasons.
Target Milestone: mozilla1.9 M9 → mozilla1.9 M10
Depends on: 399030
No longer depends on: 399030
Depends on: 394892
Target Milestone: mozilla1.9 M10 → mozilla1.9 M11
Priority: P3 → P4
Target Milestone: mozilla1.9 M11 → ---
Given that this pre-dates 1.9 and hasn't gotten wrapped up moving to wanted list..
Flags: wanted1.9+
Flags: blocking1.9-
Flags: blocking1.9+
(In reply to comment #54) > Given that this pre-dates 1.9 and hasn't gotten wrapped up moving to wanted > list.. Scrollbars used to work properly in 1.8.1 and in 1.9 pre-scrollbar rewrite; they do not now. (Other widgets, however, were never fixed properly before, so they aren't a regression.)
In summary, this is a regression for Cocoa widgets in 1.9 vs. 1.8.1. It is not a regression from Carbon in 1.8.1. In any case, this patch is still pretty much good to go for when we have the rest of our widgets cleaned up. I think the reason I originally backed it out is probably gone, now we're just waiting for new NSCell code to fix tinting.
(In reply to comment #56) > In summary, this is a regression for Cocoa widgets in 1.9 vs. 1.8.1. It is not > a regression from Carbon in 1.8.1. No, it is a regression for Carbon widgets, too; see comment 20-comment 22 and comment 25. Re-requesting blocking since it seems the rationale for denying blocking was an inadvertent view that "this never worked, not a regression."
Flags: blocking1.9- → blocking1.9?
This is a regression and a bad one, I am strongly of the opinion that this needs to be fixed. I was wrong in comment #56.
Flags: wanted1.9+
Flags: blocking1.9?
Flags: blocking1.9+
(In reply to comment #58) > This is a regression and a bad one, I am strongly of the opinion that this > needs to be fixed. I was wrong in comment #56. > K - if so needs to be higher than a p4..
Priority: P4 → P3
Depends on: 399030
This is probably not going to make it for 1.9, tough call though. Sorry for the change of opinions again, but we just don't have the time.
Flags: wanted1.9+
Flags: blocking1.9-
Flags: blocking1.9+
Attached patch josh's fix v1.0, updated to trunk (obsolete) (deleted) — Splinter Review
I've updated the patch to make it work with the current trunk code. The only difference to the original patch is in those parts of the code where we've switched to NSCell; it now looks like this: [mRadioButtonCell setEnabled:!inDisabled && frameIsInActiveWindow(aFrame)]; (In reply to comment #50) > I backed this out because I noticed that windows can get stuck in a state > where they don't redraw their controls and they always appear inactive. This issue still persists, and I think I have found the reason. The call to Invalidate() is placed in viewsWindowDidBecomeKey. However, nsWindowMap.mm only calls viewsWindowDidBecomeKey when the view doesn't implement sendFocusEvent, see http://bonsai.mozilla.org/cvsblame.cgi?file=/mozilla/widget/src/cocoa/nsWindowMap.mm&rev=1.9#201 I don't know when that's the case; during my tests I haven't been able to hit the breakpoint I placed in viewsWindowDidBecomeKey.
Attachment #277940 - Attachment is obsolete: true
I simply relocated mGeckoChild->Invalidate(PR_FALSE) to sendFocusEvent because that's always called when window focus changes. This patch works for me very well.
Attachment #307554 - Attachment is obsolete: true
Attachment #307573 - Flags: review?(joshmoz)
There is another reason my patch was backed out, aside from stuck state. Some important form controls like selects don't have correct tinting in the background and it looks awful. I'll check it out again though and see if things have improved enough to go for it.
Selects look good to me. Maybe the issue is gone.
I think I now know what you mean, but I don't think it looks awful.
If our choices here are blue scrollbars in background windows or slightly-washed-out widgets in background windows (ceteris paribus), the latter is less visually offensive, less confusing behaviorally, and more in harmony with Mac OS X. That is, one is clearly wrong, and one is just slightly imperfect.
In theory, we could avoid the washed-out look for all the widgets we draw using NSCell, using two different approaches: 1. calling setControlTint:NSClearControlTint, as suggested by Colin in comment #44, or 2. passing the right NSView to [buttonCell drawWithFrame:inView:], so that Cocoa can judge how to draw it (I think that's what Safari does). However, neither approach worked for me. So now we only do the inactive-but-not-disabled look for PushButtons using [mPushButtonCell setHighlighted:... && isInActiveWindow].
Attachment #307573 - Attachment is obsolete: true
Attachment #308973 - Flags: review?(joshmoz)
Attachment #307573 - Flags: review?(joshmoz)
Josh, what's the status of your reviewing this (or is there someone else who can look at it)?
Could someone add "ue" to the keywords? It will make it easier to find.
Comment on attachment 308973 [details] [diff] [review] fix v1.2: Make PushButtons clear instead of disabled, don't draw focus rings for inactive widgets Sorry about the review delay again, can you update this patch so it applies cleanly to current trunk?
Attachment #308973 - Flags: review?(joshmoz)
Attached patch fix v1.2, unbitrotted (obsolete) (deleted) — Splinter Review
Attachment #308973 - Attachment is obsolete: true
Attachment #314414 - Flags: review?(joshmoz)
+ if (!aFrame) return YES; Please make this two lines, return on the second line. That is convention in Cocoa widgets, happens a couple of places. Since your default value of YES is hard-coded in so many places in 'frameIsInActiveWindow', perhaps you should just make a "BOOL isActive = YES" and return that at the end. Just modify it with your logic "[nativeWindow isKeyWindow] || (windowType == eWindowType_popup)" before returning. Otherwise this looks fine to me, please post an updated patch. Might be weird to land this before we land active/inactive coloring for the unified toolbar.
Thanks for the review, I'll post an updated patch as soon as bug 406730 lands, since it modifies nsNativeThemeCocoa.mm, too.
Status: REOPENED → ASSIGNED
Assignee: joshmoz → mstange
Status: ASSIGNED → NEW
Status: NEW → ASSIGNED
Attached patch fix v1.3: address review comments (obsolete) (deleted) — Splinter Review
(In reply to comment #74) > + if (!aFrame) return YES; > > Please make this two lines, return on the second line. That is convention in > Cocoa widgets, happens a couple of places. Done. > Since your default value of YES is hard-coded in so many places in > 'frameIsInActiveWindow', perhaps you should just make a "BOOL isActive = YES" > and return that at the end. Just modify it with your logic "[nativeWindow > isKeyWindow] || (windowType == eWindowType_popup)" before returning. But then I can't early-return and end up with nested if conditions, or am I missing something?
Attachment #314414 - Attachment is obsolete: true
Attachment #319024 - Flags: superreview?(roc)
Attachment #319024 - Flags: review?(joshmoz)
Attachment #314414 - Flags: review?(joshmoz)
Comment on attachment 319024 [details] [diff] [review] fix v1.3: address review comments +static BOOL frameIsInActiveWindow(nsIFrame* aFrame) I think this function is OK but should start with a capital letter.
Attachment #319024 - Flags: superreview?(roc) → superreview+
Attachment #319024 - Flags: review?(joshmoz) → review+
Comment on attachment 319024 [details] [diff] [review] fix v1.3: address review comments Requesting approval1.9: Fixes a regression from Firefox 2 (scrollbars), very visible nativeness-issue, low risk.
Attachment #319024 - Flags: approval1.9?
Comment on attachment 319024 [details] [diff] [review] fix v1.3: address review comments a+ schrep
Attachment #319024 - Flags: approval1.9? → approval1.9+
landed
Status: ASSIGNED → RESOLVED
Closed: 17 years ago17 years ago
Resolution: --- → FIXED
I think the fix here went too far. In a recent Firefox build, form controls in background windows look disabled. In the other Tiger apps I've tested, controls just lose their blueness. The difference is easy to see with unchecked checkboxes and with the left part of <select>-like widgets.
Keywords: checkin-needed
Target Milestone: --- → mozilla1.9
I would have liked to go for the clear look instead of the disabled look but failed - see comment 67. Furthermore, there are still some widgets we draw using Carbon; these don't support the clear style. Tiger's "native" Carbon applications also use the disabled look - see for example the iTunes and Finder preferences windows.
Depends on: 432817
(In reply to comment #83) > I would have liked to go for the clear look instead of the disabled look but > failed - see comment 67. Furthermore, there are still some widgets we draw > using Carbon; these don't support the clear style. We should probably have a follow-up filed on comment 67, whether or not it's do-able now, if that's not already filed (I don't think I've seen it).
Backed out due to Ts regression, we'll probably have to fix the problems and take this for 1.9.0.x.
Status: RESOLVED → REOPENED
Flags: wanted1.9.0.x+
Resolution: FIXED → ---
Whiteboard: [missed 1.9 checkin]
Whiteboard: [missed 1.9 checkin] → [not needed for 1.9]
I would just to mention that this patch used to partially solve the problem of https://bugzilla.mozilla.org/show_bug.cgi?id=432131 since this patch now is backed out, the other bug is quite obvious again.
I tried to find the cause for the Ts regression but haven't succeeded yet. My attempts at profiling the patch didn't show any perceptible difference at all - Camino startup times stayed between 620 and 650ms, with and without patch. However, since no widgets are drawn during startup, I suspect that the cause is the ->Invalidate call in sendFocusEvent. I found out that soundFocusEvent is called quite frequently (e.g. whenever you click into the content view or when tabbing through text fields), so we're probably redrawing too often. Redraws are only necessary when the window's key state changes, so the invalidate call is probably best in nsWindowMap.mm's windowBecameKey / windowResignedKey. Placing it there also fixes bug 432131, so I've attached the patch there. It also fixes bug 432817 and bug 432866 (once this patch lands again). I suggest landing the invalidation patch first before relanding this patch so we can see where the Ts regression comes from (if there is one).
Status: REOPENED → ASSIGNED
Use Quartz Debug, turn on Autoflush Drawing, and then starting up Firefox you can see that we repaint the window several times, sometimes in weird states. You should be able to debug to find out where those redraws are coming from and why we paint in weird states sometimes.
(In reply to comment #87) > I tried to find the cause for the Ts regression but haven't succeeded yet. My > attempts at profiling the patch didn't show any perceptible difference at all - > Camino startup times stayed between 620 and 650ms, with and without patch. FWIW, half of the Firefox boxen saw no clear regression, either, and while cb-xserve01 showed the regression for Camino, (the much slower, and therefore typically more sensitive to perf changes) boxset did not. It's certainly odd that only half of our machines were showing the problem; I don't know enough about the specs and build configurations of all the Firefox perf boxen to be able to speculate intelligently on whether those might be a factor in the half/half nature of the regression.
Hardware: Macintosh → All
Target Milestone: mozilla1.9 → ---
Click-through is bug 392188. But you're right, part of this bug is related to it: The question whether controls in background windows should look transparent or disabled. I've attached a screenshot. We don't support click-through yet, so it makes sense to draw the widgets as disabled. My patch makes an exception for "push buttons" (they aren't drawn as disabled but transparent), which is kind of inconsequent... but black text on disabled buttons looks really weird.
This patch applies on top of that in bug 439354. I'll fix radiobuttons, checkboxes and selects properly in bug 394892 and bug 399030.
Attachment #319024 - Attachment is obsolete: true
Attachment #333269 - Flags: superreview?(roc)
Attachment #333269 - Flags: superreview?(roc) → superreview+
Sorry for requesting yet another review on this patch, roc, but I noticed that the previous patch resulted in disabled-looking widgets in the "Customize Toolbar" panel. The only part that has changed is this: -static NSWindow* NativeWindowForFrame(nsIFrame* aFrame, int* aLevelsUp = NULL) +static NSWindow* NativeWindowForFrame(nsIFrame* aFrame, int* aLevelsUp = NULL, + nsIWidget** aTopLevelWidget = NULL) { if (!aFrame) return nil; nsIWidget* widget = aFrame->GetWindow(); if (!widget) return nil; nsIWidget* topLevelWidget = widget->GetTopLevelWidget(aLevelsUp); + if (aTopLevelWidget) + *aTopLevelWidget = topLevelWidget; return (NSWindow*)topLevelWidget->GetNativeData(NS_NATIVE_WINDOW); +} + +static BOOL FrameIsInActiveWindow(nsIFrame* aFrame) +{ + nsIWidget* topLevelWidget = NULL; + NSWindow* win = NativeWindowForFrame(aFrame, NULL, &topLevelWidget); + if (!topLevelWidget || !win) + return YES; + + // XUL popups, e.g. the toolbar customization popup, can't become key windows, + // but controls in these windows should still get the active look. + nsWindowType windowType; + topLevelWidget->GetWindowType(windowType); + return [win isKeyWindow] || (windowType == eWindowType_popup); }
Attachment #333269 - Attachment is obsolete: true
Attachment #339651 - Flags: superreview?(roc)
Attachment #339651 - Flags: superreview?(roc) → superreview+
Status: ASSIGNED → RESOLVED
Closed: 17 years ago16 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9.1b1
Markus what about buttons? As I can see they are still not grayed-out with the latest trunk build. Take for example this Bugzilla page. All buttons e.g. Find, or Commit are not getting disabled when moving the focus to another window or application.
They're not intended to get grayed-out. I want to use the clear look instead of the disabled look because that's what native Cocoa apps do. Right now it looks a little odd because radio buttons, checkboxes and selects don't use the clear look, but the disabled look. I'll change that in bug 394892 and bug 399030. In order to be really consistent, we should also enable click-through (bug 392188).
On 10.5 when clicking on the help menu or clicking on stacks in the dock, the Firefox window remains in focus as expected but the window scrollbar and form widgets on the page are grayed until the help menu or stack is closed.
I've noticed that, too - can you file a bug, mws, and mark it blocking this one? Thanks.
Depends on: 458297
(In reply to comment #104) > I've noticed that, too - can you file a bug, mws, and mark it blocking this > one? Thanks. Ok, it's bug 458297.
Markus, there is another thing we behave differently as Safari. When Firefox loses the focus the location bar and all the search bars get a gray background. Will this issue be fixed by any of the bugs you are working on or should I create a new one?
I haven't really decided about that, but I think we could do that in a "Overhaul background window appearance" bug after bug 392188 is fixed.
Flags: wanted1.9.0.x+ → wanted1.9.0.x-
(In reply to comment #107) > I haven't really decided about that, but I think we could do that in a > "Overhaul background window appearance" bug after bug 392188 is fixed. Finally filed that as bug 471082.
Verified with Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1b3pre) Gecko/20090201 Shiretoko/3.1b3pre Ubiquity/0.1.5 ID:20090201020520
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: