As a security precaution, we have turned on the setting "Require API key authentication for API requests" for everyone. If this has broken something, please contact bugzilla-admin@mozilla.org
Last Comment Bug 714337 - quickfilter bar doesn't remember its icon button state
: quickfilter bar doesn't remember its icon button state
Status: RESOLVED FIXED
:
Product: Thunderbird
Classification: Client Software
Component: Mail Window Front End (show other bugs)
: 12 Branch
: x86_64 Windows 7
: -- normal with 1 vote (vote)
: Thunderbird 13.0
Assigned To: Mike Conley (:mconley) - PTO on Jan 20th
:
:
Mentors:
: 636445 (view as bug list)
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2011-12-30 12:09 PST by Asa Dotzler [:asa]
Modified: 2012-03-08 11:19 PST (History)
10 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
+
fixed
fixed


Attachments
quickfilter bar reverting to icon and text when it should not (47.57 KB, image/jpeg)
2011-12-30 12:09 PST, Asa Dotzler [:asa]
no flags Details
Patch v1 (1.16 KB, patch)
2012-01-09 14:01 PST, Mike Conley (:mconley) - PTO on Jan 20th
bwinton: ui‑review+
Details | Diff | Splinter Review
Patch v2 (1.56 KB, patch)
2012-01-12 12:48 PST, Mike Conley (:mconley) - PTO on Jan 20th
bugmail: review+
standard8: approval‑comm‑aurora+
standard8: approval‑comm‑beta+
Details | Diff | Splinter Review
Tests v1 (3.90 KB, patch)
2012-02-23 10:32 PST, Mike Conley (:mconley) - PTO on Jan 20th
no flags Details | Diff | Splinter Review
Tests v2 (4.12 KB, patch)
2012-02-23 10:58 PST, Mike Conley (:mconley) - PTO on Jan 20th
bugmail: review+
Details | Diff | Splinter Review
Tests v3 (r+'d by asuth) (3.72 KB, patch)
2012-02-23 12:00 PST, Mike Conley (:mconley) - PTO on Jan 20th
standard8: approval‑comm‑aurora+
standard8: approval‑comm‑beta+
Details | Diff | Splinter Review

Description User image Asa Dotzler [:asa] 2011-12-30 12:09:09 PST
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.
Comment 1 User image Blake Winton (:bwinton) (:☕️) 2011-12-30 14:45:29 PST
Mike, could this be fixed by an addition to the persistence cleanup work you did?
Comment 2 User image Mike Conley (:mconley) - PTO on Jan 20th 2012-01-04 11:29:18 PST
Unsure, but I can fiddle with it.
Comment 3 User image Maarten Brouwers 2012-01-07 11:35:56 PST
Confirmed, also on Mac OS-X
Comment 4 User image Ludovic Hirlimann [:Usul] 2012-01-09 05:53:07 PST
I just saw it on Aurora.
Comment 5 User image Mike Conley (:mconley) - PTO on Jan 20th 2012-01-09 14:01:22 PST
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.
Comment 6 User image Siddharth Agarwal [:sid0] (inactive) 2012-01-09 14:58:57 PST
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?
Comment 7 User image Mike Conley (:mconley) - PTO on Jan 20th 2012-01-10 09:58:38 PST
Sid:

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

-Mike
Comment 8 User image Blake Winton (:bwinton) (:☕️) 2012-01-11 13:07:29 PST
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. ;)
Comment 9 User image Mike Conley (:mconley) - PTO on Jan 20th 2012-01-12 06:49:06 PST
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.
Comment 10 User image Mike Conley (:mconley) - PTO on Jan 20th 2012-01-12 12:48:45 PST
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
Comment 11 User image Andrew Sutherland [:asuth] 2012-01-12 15:53:01 PST
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 :(
Comment 12 User image heyix 2012-02-23 03:51:37 PST
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?
Comment 13 User image Mike Conley (:mconley) - PTO on Jan 20th 2012-02-23 06:27:25 PST
heyix:

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

Feel like hacking on the patch?

-Mike
Comment 14 User image Mike Conley (:mconley) - PTO on Jan 20th 2012-02-23 09:21:10 PST
heyix:

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

-Mike
Comment 15 User image heyix 2012-02-23 10:15:26 PST
Hi Mike,

sounds good - many thanks.

heyix
Comment 16 User image Mike Conley (:mconley) - PTO on Jan 20th 2012-02-23 10:32:34 PST
Created attachment 600082 [details] [diff] [review]
Tests v1
Comment 17 User image Mike Conley (:mconley) - PTO on Jan 20th 2012-02-23 10:58:01 PST
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?
Comment 18 User image Asa Dotzler [:asa] 2012-02-23 11:14:18 PST
You have made my week, maybe my month. I curse this several times a day.
Comment 19 User image Mike Conley (:mconley) - PTO on Jan 20th 2012-02-23 11:16:55 PST
Comment on attachment 588159 [details] [diff] [review]
Patch v2

As discussed during Product Council, we want this for TB 11.
Comment 20 User image Mike Conley (:mconley) - PTO on Jan 20th 2012-02-23 11:17:15 PST
Comment on attachment 600093 [details] [diff] [review]
Tests v2

Same as above.
Comment 21 User image Mike Conley (:mconley) - PTO on Jan 20th 2012-02-23 11:17:42 PST
Asa:

Glad to hear it!  :D

-Mike
Comment 22 User image Mike Conley (:mconley) - PTO on Jan 20th 2012-02-23 11:37:54 PST
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.
Comment 23 User image Mike Conley (:mconley) - PTO on Jan 20th 2012-02-23 12:00:34 PST
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.
Comment 24 User image Mike Conley (:mconley) - PTO on Jan 20th 2012-02-23 12:11:56 PST
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).
Comment 25 User image Mike Conley (:mconley) - PTO on Jan 20th 2012-02-23 12:16:52 PST
Committed to comm-central as http://hg.mozilla.org/comm-central/rev/b8bdec7166ad
Comment 26 User image Mark Banner (:standard8) 2012-02-24 02:24:20 PST
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
Comment 27 User image Mike Conley (:mconley) - PTO on Jan 20th 2012-02-24 06:47:42 PST
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.
Comment 28 User image Mike Conley (:mconley) - PTO on Jan 20th 2012-02-24 06:59:31 PST
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.
Comment 29 User image Mike Conley (:mconley) - PTO on Jan 20th 2012-02-24 09:05:26 PST
The problem seems to have been resolved on our testing machines.
Comment 31 User image Wayne Mery (:wsmwk, NI for questions) 2012-03-01 13:15:14 PST
*** Bug 636445 has been marked as a duplicate of this bug. ***
Comment 32 User image ac.ekin 2012-03-08 11:14:23 PST
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?
Comment 33 User image Mike Conley (:mconley) - PTO on Jan 20th 2012-03-08 11:17:12 PST
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 User image ac.ekin 2012-03-08 11:19:29 PST
Thank you! 8-) Happy again.

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