Last Comment Bug 783899 - Thunderbird doesn't launch maximized, not full height, empty space at bottom
: Thunderbird doesn't launch maximized, not full height, empty space at bottom
Status: VERIFIED FIXED
:
Product: Thunderbird
Classification: Client Software
Component: Toolbars and Tabs (show other bugs)
: Trunk
: x86 Windows XP
: -- normal (vote)
: Thunderbird 17.0
Assigned To: Neil Deakin
:
Mentors:
: 783710 784570 (view as bug list)
Depends on:
Blocks: 743975
  Show dependency treegraph
 
Reported: 2012-08-19 11:10 PDT by Sebastian H. [:aryx][:archaeopteryx]
Modified: 2012-08-25 10:58 PDT (History)
12 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
Patch v1 (1.05 KB, patch)
2012-08-20 11:28 PDT, Mike Conley (:mconley) - (needinfo me!)
no flags Details | Diff | Review
Notify the view for resize events before the webshellwindow (1.12 KB, patch)
2012-08-22 11:42 PDT, Neil Deakin
jmathies: review+
Details | Diff | Review

Description Sebastian H. [:aryx][:archaeopteryx] 2012-08-19 11:10:07 PDT
Mozilla/5.0 (Windows NT 5.1; rv:17.0) Gecko/17.0 Thunderbird/17.0a1
Application Build ID 20120819030208

When I launch Thunderbird trunk after having it closed while being maximized, the new window shows the 'maximized' icon in the top right corner, but the window doesn't stretch to the bottom. There is some space free at the bottom, showing other windows. This is likely caused by the title bar rewrite/hack.
Comment 1 Richard Marti (:Paenglab) 2012-08-19 12:38:14 PDT
Could this be a dupe of bug 783710?

Last good 20120815
First bad 20120816

This could also point to bug 697230 like in bug 783710 suspected.

When mail.tabs.drawInTitlebar is on false then no problem. If on true the gap on the bottom looks like the height of the titlebar
Comment 2 Richard Marti (:Paenglab) 2012-08-19 12:39:09 PDT
I forgot, I tested on Win7.
Comment 3 Joe Sabash [:JoeS1] 2012-08-19 13:22:41 PDT
*** Bug 783710 has been marked as a duplicate of this bug. ***
Comment 4 Joe Sabash [:JoeS1] 2012-08-19 13:23:55 PDT
Bug shows in Winxp also
Comment 5 Bozz 2012-08-20 06:07:02 PDT
(In reply to Joe Sabash from comment #4)
> Bug shows in Winxp also

Confirmed with XP as well.
Comment 6 Mike Conley (:mconley) - (needinfo me!) 2012-08-20 10:45:04 PDT
Seeing this on Win 7. Starting investigation.
Comment 7 Mike Conley (:mconley) - (needinfo me!) 2012-08-20 11:28:05 PDT
Created attachment 653447 [details] [diff] [review]
Patch v1

So it's not clear to me *why* this patch fixes things, but removing the guard that checks that the resize event is happening on the current window seems to solve the problem.

That having been said, I don't feel great about removing a guard I don't understand.

Dao, you've worked on the Firefox version of TabsInTitlebar, and what we have is a pretty similar port.  Can you shed any light on why we're hitting this bug, why my patch fixes it, and if my patch is the way forward?

-Mike
Comment 8 Dão Gottwald [:dao] 2012-08-20 11:46:18 PDT
Comment on attachment 653447 [details] [diff] [review]
Patch v1

>-    window.addEventListener("resize", function (event) {
>+    window.addEventListener("resize", function(aEvent) {

Not sure what the point of this change is...

>-      if (event.target != window)
>-        return;
>-      TabsInTitlebar.allowedBy("sizemode", true);
>+      let sizemode = document.documentElement.getAttribute("sizemode");
>+      TabsInTitlebar.allowedBy("sizemode", sizemode == "maximized");

There are two behavior changes here. Which one actually fixes this bug?
allowedBy("sizemode", true) being called unconditionally surely doesn't seem to make sense and wasn't in line with the Firefox code, so if correcting this fixes this bug, that's good.
Comment 9 Mike Conley (:mconley) - (needinfo me!) 2012-08-20 11:47:33 PDT
(In reply to Dão Gottwald [:dao] from comment #8)
> 
> >-      if (event.target != window)
> >-        return;
> >-      TabsInTitlebar.allowedBy("sizemode", true);
> >+      let sizemode = document.documentElement.getAttribute("sizemode");
> >+      TabsInTitlebar.allowedBy("sizemode", sizemode == "maximized");
> 
> There are two behavior changes here. Which one actually fixes this bug?
> allowedBy("sizemode", true) being called unconditionally surely doesn't seem
> to make sense and wasn't in line with the Firefox code, so if correcting
> this fixes this bug, that's good.

The change that corrected the behaviour was when I removed these lines:

if (event.target != window)
  return;
Comment 10 Dão Gottwald [:dao] 2012-08-20 11:48:49 PDT
It shouldn't. If it does, there's probably something fishy going on.
Comment 11 Mike Conley (:mconley) - (needinfo me!) 2012-08-20 11:55:24 PDT
(In reply to Dão Gottwald [:dao] from comment #10)
> It shouldn't. If it does, there's probably something fishy going on.

Hrm, then yes, something fishy IS going on. :/

Not sure where to begin diagnosing it, but I'll keep poking and prodding.
Comment 12 Mike Conley (:mconley) - (needinfo me!) 2012-08-20 12:19:53 PDT
Since the regression range falls outside of any work that occurred wrt the tabs in titlebars, I have to assume that this is a regression caused by changes in mozilla-central.

Bisecting now.
Comment 13 Mike Conley (:mconley) - (needinfo me!) 2012-08-21 08:47:12 PDT
Bisecting (and necessary skipping due to broken builds) has given me the following candidate changesets:


changeset:   102463:448410c2035e
user:        Neil Deakin <neil@mozilla.com>
date:        Wed Aug 15 14:52:42 2012 -0400
summary:     Bug 743975 - use a widget listener interface instead of the remaining events that don't need an event,

changeset:   102464:4aca82dc0d41
user:        Neil Deakin <neil@mozilla.com>
date:        Wed Aug 15 14:52:42 2012 -0400
summary:     Bug 743975 - add a getpresshell method to the widget listener, r=tn

changeset:   102465:54badf5d08b0
user:        Neil Deakin <neil@mozilla.com>
date:        Wed Aug 15 14:52:42 2012 -0400
summary:     Bug 743975 - Remove events that are now unused, r=smaug

changeset:   102466:e66c290ab9fc
user:        Neil Deakin <neil@mozilla.com>
date:        Wed Aug 15 14:53:09 2012 -0400
summary:     Bug 743975 - remove the event handler argument to widget creation methods, r=tn

changeset:   102467:dfcc0507902c
user:        Neil Deakin <neil@mozilla.com>
date:        Wed Aug 15 14:53:14 2012 -0400
summary:     Bug 743975 - remove the view wrapper,r=tn
Comment 14 Alice0775 White 2012-08-21 21:26:40 PDT
*** Bug 784570 has been marked as a duplicate of this bug. ***
Comment 15 Neil Deakin 2012-08-22 11:42:59 PDT
Created attachment 654295 [details] [diff] [review]
Notify the view for resize events before the webshellwindow

Reorder these calls as there were before bug 743975.
Comment 16 Ryan VanderMeulen [:RyanVM] 2012-08-24 20:03:13 PDT
https://hg.mozilla.org/mozilla-central/rev/e048ac9eb279
Comment 17 Joe Sabash [:JoeS1] 2012-08-25 10:58:25 PDT
Verified in Mozilla/5.0 (Windows NT 5.1; rv:17.0) Gecko/17.0 Thunderbird/17.0a1 (20120825030543)
Thanks Neil!

Note You need to log in before you can comment on or make changes to this bug.