1633306 - Message filters has no scroll bar

Status: RESOLVED FIXED

(Thunderbird :: Filters, defect)

Description

[mikeleo]

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:75.0) Gecko/20100101 Firefox/75.0

Steps to reproduce:

Tools - Message filters. Select filter and click edit. If more than 21 items listed, you can't get to or see any of 22 thru the end. ver 76.0b2

Actual results:

Can't do anything with the ending filters

Expected results:

Should be able to scroll through the list or page down through it.

Comment 1

[Firefox Bug Husbandry Bot]

Because this bug's Severity has not been changed from the default since it was filed, and it's Priority is -- (non,) indicating it has has not been previously triaged, the bug's Severity is being updated to -- (default, untriaged.)

Comment 2

[Magnus Melin [:mkmelin]]

mikeleo, still see this?

Comment 3

[mikeleo]

Yes. Still the same.

Comment 4

[Wayne Mery (:wsmwk)]

I can reproduce this using 78 beta on windows 10 (remote system) with 24 filter terms - no scroll bar.
Also, the dialog size exceeds the screen size, which may be the key issue.

I cannot reproduce this on my Mac.
I did not find another report of this

Comment 5

[Alice0775 White]

Regression window:
https://hg.mozilla.org/comm-central/pushloghtml?fromchange=f760a8ee747865021a1e2acd81feeb7e2f5fcbfe&tochange=d58d6a653555e55d244c7a13beba7ba09ffa38df
https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=03fe3d76bd48bdd9be40568a136c131b1d129460&tochange=03fe3d76bd48bdd9be40568a136c131b1d129460

Comment 6

[Richard Marti (:Paenglab)]

During creating the filter terms it shows the scrollbar, but when closing it and reopening the filter edit window it goes over the screen size. This could be fixed with setting a window's max-height of something like 600px. Magnus, what do you think? Or do you have a better solution

Comment 7

[Wayne Mery (:wsmwk)]

Thanks Alice. I wonder if other things were broken by bug 1605845 ?

Comment 8

[Magnus Melin [:mkmelin]]

I don't seem to reproduce this on trunk (linux).
I wonder what would makes it different between platforms.

If we need to do something, maybe "max-height: 100vh;" could help.

Comment 9

[Wayne Mery (:wsmwk)]

(In reply to Magnus Melin [:mkmelin] from comment #8)

....
If we need to do something, maybe "max-height: 100vh;" could help.

Please assigned a resource so it it can be solved by 78.2.0

Comment 10

[Magnus Melin [:mkmelin]]

Richard, were you able to reproduce? Would comment 8 help?
I don't know what the actual bug is here that makes it Windows only - potentially some odd timing issue related to the rebuild - https://searchfox.org/comm-central/source/mail/base/content/FilterListDialog.js#748

Comment 11

[Richard Marti (:Paenglab)]

(In reply to Magnus Melin [:mkmelin] from comment #10)

Richard, were you able to reproduce? Would comment 8 help?
I don't know what the actual bug is here that makes it Windows only - potentially some odd timing issue related to the rebuild - https://searchfox.org/comm-central/source/mail/base/content/FilterListDialog.js#748

Yes, with enough rules in the filter, opening it makes the dialog taller than the screen. Comment 8 on the <window> doesn't help, then the dialog opens in its minimal height.

When I apply comment 8 to the #searchTermBox the dialog open with around 2/3 screen height but let me make it taller. Should we do this?

Comment 12

[Magnus Melin [:mkmelin]]

I still find it strange I don't see it on linux with lots of rules. If that works lets do it.

Comment 15

[Wayne Mery (:wsmwk)]

Proactively assigning, since you sounded interested. If it's available real soon it can make the beta.

Comment 16

[Richard Marti (:Paenglab)]

Attachment: 1633306-filterEditor-height.patch

With max-height: 100vh; on #searchTermBox.

Comment 17

[Magnus Melin [:mkmelin]]

Comment on attachment 9168416 [details] [diff] [review]
1633306-filterEditor-height.patch

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

::: mailnews/base/search/content/searchTerm.inc.xhtml
@@ +12,5 @@
>                     accesskey="&matchAllMsgs.accesskey;"/>
>            </radiogroup>
>  
> +          <hbox id="searchTermBox" flex="1"
> +                style="min-height:100px; max-height: 100vh;">

I'd prefer for inline style values, no space after colons, only after semicolon

Comment 18

[Magnus Melin [:mkmelin]]

Please update the commit message as to why we make this change

Comment 19

[Richard Marti (:Paenglab)]

Attachment: 1633306-filterEditor-height.patch

Addressed the review comments.

[Approval Request Comment]
User impact if declined: filter editor can expand over the screen size with many rules in it
Testing completed (on c-c, etc.): soon on c-c
Risk to taking this patch (and alternatives if risky): low

Comment 20

[Wayne Mery (:wsmwk)]

Comment on attachment 9168478 [details] [diff] [review]
1633306-filterEditor-height.patch

[Triage Comment]
Approved for beta

Comment 21

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/e3cf8ee4d78b
Limit the height of the Filter editor to not expand over the screen size. r=mkmelin

Comment 23

[Rob Lemley [:rjl]]

Thunderbird 80.0b2:
https://hg.mozilla.org/releases/comm-beta/rev/841460aac889

Comment 24

[Wayne Mery (:wsmwk)]

Comment on attachment 9168478 [details] [diff] [review]
1633306-filterEditor-height.patch

[Triage Comment]
Approved for esr78

Comment 25

[Geoff Lankow (:darktrojan)]

Thunderbird 78.1.2:
https://hg.mozilla.org/releases/comm-esr78/rev/284138c7cea0

Comment 26

[Matt]

Attachment: FIlterWindow

Looks like this fix did not actually fix the issue. Perhaps it is related to me having a dual monitor setup and Thunderbird on a smaller secondary monitor. But clearly not fixed in 78.3 on Windows 10.

As an aside creating this filter following https://support.mozilla.org/en-US/questions/1316823 I found the focus sent to the next line after pressing + than went somewhere else before I could actually input text.

What additional is needed here?

Comment 27

[Richard Marti (:Paenglab)]

Could you test what happens when TB is on the bigger screen? Maybe we should check through JS the screen size and the set the max-height. But this should be done in a new bug as this one is already closed since 4 month.