Closed Bug 402627 Opened 15 years ago Closed 15 years ago

"WARNING: Calling SetWindowTitlebarColor on window that isn't of the ToolbarWindow class."

Categories

(Core :: Widget: Cocoa, defect)

x86
macOS
defect
Not set
minor

Tracking

()

RESOLVED FIXED

People

(Reporter: jruderman, Assigned: cbarrett)

References

Details

(Keywords: regression, Whiteboard: [not a blocker; needs explicit a+])

Attachments

(2 files, 3 obsolete files)

This warning appears when I start Firefox:

WARNING: Calling SetWindowTitlebarColor on window that isn't of the ToolbarWindow class.: file /Users/jruderman/trunk/mozilla/widget/src/cocoa/nsCocoaWindow.mm, line 1089

I'm guessing this started happening with the landing of bug 402125.
Can you break on that line and backtrace? 
Attached file stack trace
Assignee: joshmoz → cbarrett
This is caused by someone trying to set the titlebar color of the hidden window.

They should probably stop doing that ;)
I think you better fail silently, otherwise we'd have to distinguish the hidden window any anything which might be opened as a sheet using pure css selectors...
Attached patch fix v1.0 (obsolete) — Splinter Review
If we're the hidden window, we should fail silently according to Mano.
Attachment #287616 - Flags: review?(joshmoz)
Comment on attachment 287616 [details] [diff] [review]
fix v1.0

+  // Fail silently for the hidden window.
+  if (SameCOMIdentity(GetHiddenWindowWidget(), (nsIWidget*)this))
+    return NS_ERROR_FAILURE;

I'm not sure raising an error here is failing silently, and in any case the comment itself is saying nothing new about obvious code so it isn't necessary. Why return an error instead of NS_OK? I'm not sure which is better, error or NS_OK.
Attached patch fix v1.1 (obsolete) — Splinter Review
This should be a little clearer.
Attachment #287616 - Attachment is obsolete: true
Attachment #287701 - Flags: review?(joshmoz)
Attachment #287616 - Flags: review?(joshmoz)
Comment on attachment 287701 [details] [diff] [review]
fix v1.1

+  nsIWidget* hiddenWindowWidgetNoCOMPtr = GetHiddenWindowWidget();
   return static_cast<nsCocoaWindow*>(hiddenWindowWidgetNoCOMPtr)->GetMenuBar();  

If GetHiddenWindowWidget return null this will crash.
Attachment #287701 - Flags: review?(joshmoz) → review-
Attached patch fix v1.2 (obsolete) — Splinter Review
Thanks, fixed.
Attachment #287701 - Attachment is obsolete: true
Attachment #289100 - Flags: review?(joshmoz)
Comment on attachment 289100 [details] [diff] [review]
fix v1.2

>@@ -1521,6 +1533,7 @@ void patternDraw(void* aInfo, CGContextR
>     CGColorSpaceRelease(colorSpace);
>     CGFunctionRelease(function);
>     CGContextDrawShading(aContext, shading);
>+    CGShadingRelease(shading);

This won't apply and it has nothing to do with this bug.
Attachment #289100 - Flags: review?(joshmoz) → review-
Comment on attachment 289100 [details] [diff] [review]
fix v1.2

>+    // Don't output a warning for the hidden window.
>+    if (!SameCOMIdentity(GetHiddenWindowWidget(), (nsIWidget*)this))
>+      NS_WARNING("Calling SetWindowTitlebarColor on window that isn't of the ToolbarWindow class.");
>     return NS_ERROR_FAILURE;
>   }

Since NS_WARNING will only apply in a debug build, I think you should also make the if-statement only run on debug builds by putting it inside #if DEBUG.
Attached patch fix v1.3Splinter Review
Should apply cleanly now that I'm not working from an old source tree. Using NS_WARN_IF_FALSE as well now.
Attachment #289100 - Attachment is obsolete: true
Attachment #290286 - Flags: review?(joshmoz)
Attachment #290286 - Flags: superreview?(roc)
Attachment #290286 - Flags: review?(joshmoz)
Attachment #290286 - Flags: review+
Whiteboard: [not a blocker; needs explicit a+]
Attachment #290286 - Flags: superreview?(roc) → superreview+
Comment on attachment 290286 [details] [diff] [review]
fix v1.3

Want this to land for 1.9. It's silencing a spurious warning during start up, and helps make automated testing easier and real warnings stand out better.
Attachment #290286 - Flags: approval1.9?
Attachment #290286 - Flags: approval1.9? → approval1.9+
Checking in widget/src/cocoa/nsCocoaWindow.mm;
/cvsroot/mozilla/widget/src/cocoa/nsCocoaWindow.mm,v  <--  nsCocoaWindow.mm
new revision: 1.120; previous revision: 1.119
done
Status: NEW → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
Flags: in-testsuite-
You need to log in before you can comment on or make changes to this bug.