Closed Bug 1633306 Opened 2 years ago Closed 2 years ago

Message filters has no scroll bar

Categories

(Thunderbird :: Filters, defect)

Unspecified
Windows
defect

Tracking

(thunderbird_esr78+ fixed, thunderbird78 wontfix, thunderbird80 fixed)

RESOLVED FIXED
81 Branch
Tracking Status
thunderbird_esr78 + fixed
thunderbird78 --- wontfix
thunderbird80 --- fixed

People

(Reporter: mikeleo, Assigned: Paenglab)

References

(Regressed 1 open bug, Regression)

Details

(Keywords: regression)

Attachments

(2 files, 1 obsolete file)

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.

Keywords: regression

mikeleo, still see this?

Flags: needinfo?(mikeleo)

Yes. Still the same.

Flags: needinfo?(mikeleo)

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

Status: UNCONFIRMED → NEW
Ever confirmed: true
Flags: needinfo?(richard.marti)
Summary: Message filters do not scroll or page. Can't see any after the 21st. 76.0b2 → Message filters has no scroll bar

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

Flags: needinfo?(richard.marti) → needinfo?(mkmelin+mozilla)

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

Severity: -- → S2
Regressed by: 1605845

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.

Flags: needinfo?(mkmelin+mozilla)
OS: Unspecified → Windows

(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

Flags: needinfo?(mkmelin+mozilla)

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

(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?

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

Flags: needinfo?(mkmelin+mozilla)
Duplicate of this bug: 1657424
Duplicate of this bug: 1657507

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

Assignee: nobody → richard.marti
Flags: needinfo?(richard.marti)

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

Flags: needinfo?(richard.marti)
Attachment #9168416 - Flags: review?(mkmelin+mozilla)
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
Attachment #9168416 - Flags: review?(mkmelin+mozilla) → review+

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

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

Attachment #9168416 - Attachment is obsolete: true
Attachment #9168478 - Flags: review+
Attachment #9168478 - Flags: approval-comm-esr78?
Attachment #9168478 - Flags: approval-comm-beta?

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

[Triage Comment]
Approved for beta

Attachment #9168478 - Flags: approval-comm-beta? → approval-comm-beta+
Target Milestone: --- → 81 Branch

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

Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Duplicate of this bug: 1657923

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

[Triage Comment]
Approved for esr78

Attachment #9168478 - Flags: approval-comm-esr78? → approval-comm-esr78+
Attached image 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?

Flags: needinfo?(richard.marti)

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.

Flags: needinfo?(richard.marti)
Regressions: 1682139
You need to log in before you can comment on or make changes to this bug.