986908 - Lightning modifies Thunderbird's min-width when installed (failing TB mozmill tests)

Status: RESOLVED FIXED

(Calendar :: Calendar Frontend, defect)

Description

[Philipp Kewisch [:Fallen] [:📆][:🧩]]

Without Lightning installed, Thunderbird's main window minimal width is about XXX px. With Lightning installed, its 721 px.

This causes at least these two mozmill tests to fail:
* quick-filter-bar/test-display-issues.js: Attempts to resize to 600px
* attachment/test-attachment-menus.js: different width causes click on 
  wrong element

Investigating Lightning's XUL, at least these contribute to the minimal width. They might not have an explicit min-width, but they influence sizing:
* The #calendar-nav-control bar
* The calendar sidebar
* The unifinder's #event-filter-menulist

The sidebar limiting the min-width is normal, the Thunderbird sidebar does the same. Its sort of strange that a non-visible sidebar (i.e when mail tab is selected) will also limit the min-width, on the other hand the window jumping sizes when tabs are switched is also not great.

I see two ways to fix this. The easy way would be to modify Thunderbird's mozmill tests to not assume the window can be resized to widths lower than 800px. We could even enforce this using a min-width on the window, but this could be troublesome for users that hide the folder pane and just want a very compact list of email.

The next option would be some sort of fix in core layout that changes the way sizing is calculated so that nonvisible elements don't contribute to the window's min-width. This might have a lot of side effects though and I'm not even sure its wanted.

The last option would be to do something in Lightning to lower the min-width, at least when Lightning is not visible. This could be anything from fiddling with the XUL elements so that they no longer contribute to the min-width to temporarily collapsing elements when the calendar tab is not visible.

Personally I would go with the easiest solution, changing the mozmill tests to assume that the minimum width is 800px.

Comment 1

[Philipp Kewisch [:Fallen] [:📆][:🧩]]

Attachment: Fix in Lightning - v1

(In reply to Philipp Kewisch [:Fallen] from comment #0)
> Without Lightning installed, Thunderbird's main window minimal width is
> about XXX px. With Lightning installed, its 721 px.
Actually, with the folder pane collapsed its:

* 921px with Lightning, Today Pane showing
* 721px with Lightning, Today pane hidden
* 524px with Lightning, Today pane hidden in all modes, calendar pane hidden
* 200px without Lightning

The fluctuations here make me think its best to just fix this in Lightning now, this way the state of the panes won't influence the TB mozmill tests. The failing tests I've looked at until now work fine even with the today pane visible.

Comment 2

[Philipp Kewisch [:Fallen] [:📆][:🧩]]

Here is a succeeding try run with this patch and a few others related to Lightning packaged-tests:

https://tbpl.mozilla.org/?tree=Thunderbird-Try&rev=cedcf097ac8d

Comment 3

[Philipp Kewisch [:Fallen] [:📆][:🧩]]

Comment on attachment 8395383 [details] [diff] [review]
Fix in Lightning - v1

I think this is ready for review. Magnus, can you review the tabmail.xml part? You did a few tabmail.xml reviews lately and mconley's queue is already very long.

Comment 4

[Magnus Melin [:mkmelin]]

Comment on attachment 8395383 [details] [diff] [review]
Fix in Lightning - v1

Review of attachment 8395383 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good to me! r=mkmelin

::: calendar/lightning/themes/windows/lightning.css
@@ +2,3 @@
>   * License, v. 2.0. If a copy of the MPL was not distributed with this
>   * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
>   

nit: trailing space

::: mail/base/content/tabmail.xml
@@ +551,4 @@
>            else if (!background)
>              this.panelContainer.selectedPanel = tab.mode.tabType.panel;
>  
> +          // Make sure the new panel is marked selected

nit: make it a proper sentence (add .)

@@ +1117,5 @@
>  
>                this.panelContainer.selectedPanel = tab.panel ||
>                                                    tab.mode.tabType.panel;
>  
> +              // update the selected attribute on the current and old tab panel

here too

Comment 5

[Mohit Kanwal [:redDragon]]

Comment on attachment 8395383 [details] [diff] [review]
Fix in Lightning - v1

Review of attachment 8395383 [details] [diff] [review]:
-----------------------------------------------------------------

Apart from some style related fixes looks good to me.

Comment 6

[Philipp Kewisch [:Fallen] [:📆][:🧩]]

Attachment: Fix - v2

Patch for checkin, thanks for the review comments.

Comment 7

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/comm-central/rev/86ee54b4f7bb