Closed Bug 714337 Opened 9 years ago Closed 9 years ago
quickfilter bar doesn't remember its icon button state
47.57 KB, image/jpeg
1.56 KB, patch
|Details | Diff | Splinter Review|
3.72 KB, patch
|Details | Diff | Splinter Review|
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
Confirmed, also on Mac OS-X
I just saw it on Aurora.
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.
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.
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 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 :(
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
Hi Mike, sounds good - many thanks. heyix
These tests pass for me when Patch v2 is applied, and fail when it's reverted. Look ok?
9 years ago
Attachment #588159 - Flags: review?(bugmail)
9 years ago
Attachment #588159 - Flags: review?(bugmail) → review+
Attachment #600093 - Flags: review?(bugmail) → review+
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.
Comment on attachment 600093 [details] [diff] [review] Tests v2 Same as above.
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.
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).
Committed to comm-central as http://hg.mozilla.org/comm-central/rev/b8bdec7166ad
Status: NEW → RESOLVED
Closed: 9 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.
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
Thank you! 8-) Happy again.
You need to log in before you can comment on or make changes to this bug.