quickfilter bar doesn't remember its icon button state

RESOLVED FIXED in Thunderbird 13.0

Status

Thunderbird
Mail Window Front End
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: asa, Assigned: mconley)

Tracking

12 Branch
Thunderbird 13.0
x86_64
Windows 7

Thunderbird Tracking Flags

(thunderbird11+ fixed, thunderbird12 fixed)

Details

Attachments

(3 attachments, 3 obsolete attachments)

(Reporter)

Description

6 years ago
Created attachment 585016 [details]
quickfilter bar reverting to icon and text when it should not

I run in the vertical view layout and the quickfilter bar often fails to remember that its buttons should be icons and not icons and labels. This pushes the filter box out of view some of the time. It's really annoying and I cannot figure out what triggers it. Every time I restart Thunderbird, I have to fiddle with the width of the message pane to re-trigger the icon only mode to get the filter text box back in view. See screenshot.
Mike, could this be fixed by an addition to the persistence cleanup work you did?
Unsure, but I can fiddle with it.
Assignee: nobody → mconley

Comment 3

6 years ago
Confirmed, also on Mac OS-X
I just saw it on Aurora.
Created attachment 587121 [details] [diff] [review]
Patch v1

Wow, that was simpler than expected.

So, the QFB buttons default to full icons and text when TB starts up fresh for the first time.  If the window is shrunk enough, a "shrunk" attribute on the element holding the buttons is set to true.

We're now persisting that flipped attribute.  I tried it locally on Win 7, and it seemed to work alright.
Attachment #587121 - Flags: ui-review?(bwinton)
Attachment #587121 - Flags: review?(sagarwal)
Why does this bug not show up with the default classic message view even after the window's been shrunk and Thunderbird's restarted, and why can't we do the same thing?
Sid:

Hm, using the classic message view, I'm still seeing the problem in TB 11.

-Mike
Comment on attachment 587121 [details] [diff] [review]
Patch v1

(I don't _think_ we need ui-review for making Thunderbird start up in the same state that it closed in, but I'll say ui-r=me, if it makes you feel better. ;)
Attachment #587121 - Flags: ui-review?(bwinton) → ui-review+
Comment on attachment 587121 [details] [diff] [review]
Patch v1

Hm, so after testing this for a few days, I keep seeing the bug crop up.

So, not as easy as I originally thought.  I'll keep digging.
Attachment #587121 - Flags: review?(sagarwal)
Created attachment 588159 [details] [diff] [review]
Patch v2

So it looks like an attempt was already made to fix this problem, but we were calling "node.onoverflow();", and "node.onresize()", which I think is not actually possible.

Thus, the situation this bug describes also results in an error in the Error Console stating, "node.onoverflow is not a function".

The was the best solution I could find to fire the onoverflow and onresize events.  It sure beats eval.

asuth:  Am I overlooking anything here?

-Mike
Attachment #587121 - Attachment is obsolete: true
Attachment #588159 - Flags: feedback?(bugmail)
Comment on attachment 588159 [details] [diff] [review]
Patch v2

If we can't call onresize(), will the test for 'onresize' work?

I think this should be promoted to mozmill tested since this is fairly voodoo and I thought I had done it correctly in the first place (but apparently I did not, or something regressed it, etc.)  The tests for the on-the-fly stuff happens in:
mail/test/mozmill/quick-filter-bar/test-display-issues.js

I think you should be able to add a test that creates a new 3-pane window of a given size and make sure the initial state is correct (after waiting for our hideous sleep to complete :(
Attachment #588159 - Flags: feedback?(bugmail)

Comment 12

6 years ago
I've got the same issue - and find it really annoying like asa. I see it's still in progress. Will it be fixed in TB 11?
heyix:

Unfortunately, no, I don't think this patch will make it for TB 11.

Feel like hacking on the patch?

-Mike
heyix:

I spoke too soon!  I've just been re-prioritized onto this bug. :)

-Mike

Comment 15

6 years ago
Hi Mike,

sounds good - many thanks.

heyix
Created attachment 600082 [details] [diff] [review]
Tests v1
Created attachment 600093 [details] [diff] [review]
Tests v2

These tests pass for me when Patch v2 is applied, and fail when it's reverted.

Look ok?
Attachment #600082 - Attachment is obsolete: true
Attachment #600093 - Flags: review?(bugmail)
Attachment #588159 - Flags: review?(bugmail)
tracking-thunderbird11: --- → ?
Attachment #588159 - Flags: review?(bugmail) → review+
Attachment #600093 - Flags: review?(bugmail) → review+
(Reporter)

Comment 18

6 years ago
You have made my week, maybe my month. I curse this several times a day.
Comment on attachment 588159 [details] [diff] [review]
Patch v2

As discussed during Product Council, we want this for TB 11.
Attachment #588159 - Flags: approval-comm-beta?
Attachment #588159 - Flags: approval-comm-aurora?
Comment on attachment 600093 [details] [diff] [review]
Tests v2

Same as above.
Attachment #600093 - Flags: approval-comm-beta?
Attachment #600093 - Flags: approval-comm-aurora?
Asa:

Glad to hear it!  :D

-Mike
Comment on attachment 600093 [details] [diff] [review]
Tests v2

These tests fail on Linux because when seeing if the icons are expanded if the message pane is small.  This is because it seems that our threshold for "small" is different on Linux than on OSX or Windows, and I believe we're restricted on the resolution for our Linux test builders.

The interesting test case is for when the window spawns, and the message pane is *large*, and ensuring that the icons are collapsed.  So I think, due to time constraints, I am going to remove the superfluous test where we ensure that the icons expand if the message pane is small.

So, for now, reverting my a? requests until I have the new patch up.
Attachment #600093 - Flags: approval-comm-beta?
Attachment #600093 - Flags: approval-comm-aurora?
Created attachment 600124 [details] [diff] [review]
Tests v3 (r+'d by asuth)

Removing uninteresting and problematic-for-Linux test case.  Carrying forward r+ from asuth.

I'll run these tests locally again on my Windows / OSX boxes before re-requesting approval for aurora/beta.
Attachment #600093 - Attachment is obsolete: true
Comment on attachment 600124 [details] [diff] [review]
Tests v3 (r+'d by asuth)

These tests act as suspected, and now pass on all three platforms (locally).
Attachment #600124 - Flags: approval-comm-beta?
Attachment #600124 - Flags: approval-comm-aurora?
Committed to comm-central as http://hg.mozilla.org/comm-central/rev/b8bdec7166ad
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 13.0
I think this broke some MozMill tests on Mac 10.6:

TEST-UNEXPECTED-FAIL | /builds/slave/comm-central-macosx64-opt-unittest-mozmill/build/mozmill/attachment/test-attachment-menus.js | test-attachment-menus.js::test_multiple_attachments_one_detached
TEST-UNEXPECTED-FAIL | /builds/slave/comm-central-macosx64-opt-unittest-mozmill/build/mozmill/attachment/test-attachment-menus.js | test-attachment-menus.js::test_multiple_attachments_all_detached
TEST-UNEXPECTED-FAIL | (runtestlist.py) | Exited with code 1 during directory run
TEST-UNEXPECTED-FAIL | /builds/slave/comm-central-macosx64-opt-unittest-mozmill/build/mozmill/message-header/test-message-header.js | test-message-header.js::test_toolbar_collapse_and_expand
TEST-UNEXPECTED-FAIL | (runtestlist.py) | Exited with code 1 during directory run
TEST-UNEXPECTED-FAIL | /builds/slave/comm-central-macosx64-opt-unittest-mozmill/build/mozmill/quick-filter-bar/test-display-issues.js | test-display-issues.js::test_buttons_collapse_and_expand
TEST-UNEXPECTED-FAIL | /builds/slave/comm-central-macosx64-opt-unittest-mozmill/build/mozmill/quick-filter-bar/test-display-issues.js | test-display-issues.js::test_buttons_collapse_and_expand_on_spawn_in_vertical_mode
TEST-UNEXPECTED-FAIL | /builds/slave/comm-central-macosx64-opt-unittest-mozmill/build/mozmill/quick-filter-bar/test-display-issues.js | test-display-issues.js::teardownModule
TEST-UNEXPECTED-FAIL | (runtestlist.py) | Exited with code 1 during directory run
TEST-UNEXPECTED-FAIL | /builds/slave/comm-central-macosx64-opt-unittest-mozmill/build/mozmill/session-store/test-session-store.js | test-session-store.js::test_message_pane_width_persistence
TEST-UNEXPECTED-FAIL | (runtestlist.py) | Exited with code 1 during directory run
It seems to be intermittent.  I'm working with jhopkins now to determine if one of the Mac 10.6 test machines has an incorrect screen resolution.
Alright, looks like we had some screen resolution discrepancies.  jhopkins is on the case to fix that.

I'll wait for a few more builds to land on trunk today to make sure that the problem is resolved.
The problem seems to have been resolved on our testing machines.
Attachment #588159 - Flags: approval-comm-beta?
Attachment #588159 - Flags: approval-comm-beta+
Attachment #588159 - Flags: approval-comm-aurora?
Attachment #588159 - Flags: approval-comm-aurora+
Attachment #600124 - Flags: approval-comm-beta?
Attachment #600124 - Flags: approval-comm-beta+
Attachment #600124 - Flags: approval-comm-aurora?
Attachment #600124 - Flags: approval-comm-aurora+
Landed on branches:

http://hg.mozilla.org/releases/comm-aurora/rev/2fd019398510
http://hg.mozilla.org/releases/comm-beta/rev/0247b9da3bd1
status-thunderbird11: --- → fixed
status-thunderbird12: --- → fixed
tracking-thunderbird11: ? → +
Duplicate of this bug: 636445

Comment 32

6 years ago
I wonder when the end users can expect to see this resolved? Not knowing any better, I started this on the following location:
http://bit.ly/xCEAL1

Someone suggested that I should report it here. I see it as "resolved" but obviously not for me. Is the ETA Thunderbird 13? Really?
ac.ekin:

Hey - this was patched for Thunderbird 13, but then backported to Thunderbird 12 (EarlyBird) and Thunderbird 11 (Beta).

So this fix should be in your hands when Thunderbird 11 is released next week.

Or, if you can't wait that long, you can try Beta or EarlyBird:

http://www.mozilla.org/en-US/thunderbird/channel/

All the best,

-Mike

Comment 34

6 years ago
Thank you! 8-) Happy again.
You need to log in before you can comment on or make changes to this bug.