522217 - [Mac] Missing window title after leaving the full screen mode

Status: VERIFIED FIXED

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

Description

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.2b1pre) Gecko/20091011 Namoroka/3.6b1pre ID:20091011033822

When you leave the full screen mode on OS X the window title is not restored and is shown as blank as long as the windows are open. Someone has to open a new tab to show the title again.

Steps:
1. Open a window with a couple of tabs
2. Enter full screen mode and leave it immediately
3. Switch between the open tabs
4. Open a new tab

After step 2 none of the open tabs show a window title even when switching through them. With step 4 the title is restored for each tab.

Comment 1

[Damon Sicore (:damons)]

I don't see this in:

Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.6; en-US; rv:1.9.2b1pre) Gecko/20091011 Namoroka/3.6b1pre ID: 20091011033822

Comment 2

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Still there with Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.2b2pre) Gecko/20091022 Namoroka/3.6b2pre ID:20091022033608

Could this be a 10.5 only bug and 10.6 is not affected? Marcia, could you test this on a lab machine?

Comment 3

[Markus Stange [:mstange]]

Looking at the code, this can't really depend on whether you use 10.5 or 10.6; the code is just wrong - it doesn't save the title of the window before destroying it.

Damon, maybe you tested this with a fullscreen video instead of fullscreening the browser window?

Comment 4

[Markus Stange [:mstange]]

Attachment: wip

Comment 5

[Johnathan Nightingale [:johnath]]

This blocks bug 521744, which is a blocker; ergo, this is a blocker.

Comment 6

[Markus Stange [:mstange]]

Attachment: v1

This isn't beautiful, but I think it's what we need to do. This fixes two bugs:
 - State that's only saved on the NSWindow wasn't re-imported into the new
   window.
 - In fullscreen mode, our window wasn't a ToolbarWindow, so setting titlebar
   colors or drawintitlebar didn't work on it.

This patch moves the ownership of all that state into a common base class, "BaseWindow", so that things like titlebar colors can even be set on a BorderlessWindow.

Comment 7

[Markus Stange [:mstange]]

Tests passed on tryserver.

Comment 8

[Josh Aas]

Comment on attachment 408886 [details] [diff] [review]
v1

>+- (void)importState:(NSDictionary*)aState
>+{
>+  [self setTitle:[aState objectForKey:@"title"]];
>+  [self setDrawsContentsIntoWindowFrame:[[aState objectForKey:@"drawsContentsIntoWindowFrame"] boolValue]];
>+  [self setTitlebarColor:[aState objectForKey:@"activeTitlebarColor"] forActiveWindow:YES];
>+  [self setTitlebarColor:[aState objectForKey:@"inactiveTitlebarColor"] forActiveWindow:NO];
>+}
>+
>+- (NSMutableDictionary*)exportState
>+{
>+  NSMutableDictionary* state = [NSMutableDictionary dictionaryWithCapacity:10];
>+  [state setObject:[self title] forKey:@"title"];
>+  [state setObject:[NSNumber numberWithBool:[self drawsContentsIntoWindowFrame]]
>+            forKey:@"drawsContentsIntoWindowFrame"];
>+  NSColor* activeTitlebarColor = [self titlebarColorForActiveWindow:YES];
>+  if (activeTitlebarColor) {
>+    [state setObject:activeTitlebarColor forKey:@"activeTitlebarColor"];
>+  }
>+  NSColor* inactiveTitlebarColor = [self titlebarColorForActiveWindow:NO];
>+  if (inactiveTitlebarColor) {
>+    [state setObject:inactiveTitlebarColor forKey:@"inactiveTitlebarColor"];
>+  }
>+  return state;

Please make the strings used here static variables. That way we won't get bitten by any typos. Post a new patch with that, r+. Thanks!

Comment 9

[Markus Stange [:mstange]]

Attachment: v2

Comment 10

[Markus Stange [:mstange]]

http://hg.mozilla.org/mozilla-central/rev/8ed663f0f663

Comment 11

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Looks good in Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.3a1pre) Gecko/20091104 Minefield/3.7a1pre ID:20091104031046

Thanks Markus.

Comment 12

[Markus Stange [:mstange]]

http://hg.mozilla.org/releases/mozilla-1.9.2/rev/a130c9120ad0

Comment 13

[Markus Stange [:mstange]]

Attachment: 1.9.2 patch

merged around bug 513158

Comment 14

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Verified fixed on 1.9.2 with Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.2b2pre) Gecko/20091106 Namoroka/3.6b2pre ID:20091106033745