Last Comment Bug 824778 - Add bottom border to tab toolbar
: Add bottom border to tab toolbar
Status: RESOLVED FIXED
:
Product: Thunderbird
Classification: Client Software
Component: Theme (show other bugs)
: unspecified
: All Linux
: -- normal (vote)
: Thunderbird 22.0
Assigned To: Richard Marti (:Paenglab)
:
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-12-26 11:42 PST by Richard Marti (:Paenglab)
Modified: 2013-03-26 07:43 PDT (History)
2 users (show)
standard8: in‑testsuite+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
proposed fix (4.09 KB, patch)
2012-12-26 11:49 PST, Richard Marti (:Paenglab)
no flags Details | Diff | Splinter Review
unbitrotted patch (4.08 KB, patch)
2013-01-21 10:11 PST, Richard Marti (:Paenglab)
bugs: review+
bugs: ui‑review+
Details | Diff | Splinter Review
comparison (108.60 KB, image/png)
2013-03-18 10:33 PDT, Richard Marti (:Paenglab)
no flags Details
patch passing mozmill test (4.82 KB, patch)
2013-03-21 15:08 PDT, Richard Marti (:Paenglab)
mconley: review+
richard.marti: ui‑review+
Details | Diff | Splinter Review

Description Richard Marti (:Paenglab) 2012-12-26 11:42:50 PST
When we introduced the Australis tabs on Linux we haven't drawn a bottom border because some Linus themes like Clearlooks draw their own borders. This double borders then would looked weird.

I've found now a solution to use our own border.
Comment 1 Richard Marti (:Paenglab) 2012-12-26 11:49:37 PST
Created attachment 695805 [details] [diff] [review]
proposed fix

My solution is a negative top margin to remove the Linux theme border and add 1px padding to have still the same height.

I've also added new tab graphics to remove the darker part where the tab border and the toolbar border come together.
Comment 2 Richard Marti (:Paenglab) 2013-01-21 10:11:07 PST
Created attachment 704589 [details] [diff] [review]
unbitrotted patch
Comment 3 Andreas Nilsson (:andreasn) 2013-03-18 06:11:44 PDT
I don't currently have a running build. Could you post a screenshot?
Comment 4 Richard Marti (:Paenglab) 2013-03-18 10:33:25 PDT
Created attachment 726226 [details]
comparison

The two on top are Clearlooks-Phenix and Mint-x with actual look.

The two on bottom are the same with patch applied.
Comment 5 Andreas Nilsson (:andreasn) 2013-03-19 05:33:59 PDT
Comment on attachment 704589 [details] [diff] [review]
unbitrotted patch

UI and code looks good!
Approved!
Comment 6 Ryan VanderMeulen [:RyanVM] 2013-03-19 14:50:55 PDT
https://hg.mozilla.org/comm-central/rev/9ff930f2836c
Comment 7 Mark Banner (:standard8, limited time in Dec) 2013-03-20 07:01:15 PDT
I backed this out due to unit test failures:

https://hg.mozilla.org/comm-central/rev/47d10a58fa47

https://tbpl.mozilla.org/php/getParsedLog.php?id=20879665&tree=Thunderbird-Trunk#error5

TEST-START | /home/cltbld/talos-slave/test/build/mozmill/folder-display/test-opening-messages.js | test_open_message_in_new_window
Step Pass: {"function": "Controller.keypress()"}
Test Failure: messenger window height not equal to the sum of children heights: "stringbundleset": 0, "": 0, "": 0, "": 0, "": 0, "": 0, "": 0, "": 0, "": 0, "": 0, "": 0, "": 0, "": 0, "": 0, "": 0, "": 0, "": 0, "": 0, "mailCommands": 0, "mailKeys": 0, "": 0, "attachmentListContext": 0, "attachmentItemContext": 0, "header-toolbar-context-menu": 0, "attachment-toolbar-context-menu": 0, "copyUrlPopup": 0, "toolbar-context-menu": 0, "navigation-toolbox": 22, "mail-toolbox": 38, "aHTMLTooltip": 0, "messagepaneboxwrapper": 1045, "customizeToolbarSheetPopup": 0, "status-bar": 22, "": 0, "": 0, "": 0, "": 0, "": 0, "": 0, "": 0, "": 0, "": 0, "": 0, "": 0, "": 0, "": 0, "": 0, "": 0, "": 0, "": 0, "": 0, "": 0, "": 0, "": 0, "": 0, "": 0, "": 0, "": 0, "": 0, "": 0, "": 0, "": 0, : '1126' != '1127'.
TEST-UNEXPECTED-FAIL | /home/cltbld/talos-slave/test/build/mozmill/folder-display/test-opening-messages.js | test-opening-messages.js::test_open_message_in_new_window
TEST-START | /home/cltbld/talos-slave/test/build/mozmill/folder-display/test-opening-messages.js | test_open_message_in_existing_window
Step Pass: {"function": "Controller.keypress()"}
Test Failure: Timed out waiting for window open!
TEST-UNEXPECTED-FAIL | /home/cltbld/talos-slave/test/build/mozmill/folder-display/test-opening-messages.js | test-opening-messages.js::test_open_message_in_existing_window
Comment 8 Richard Marti (:Paenglab) 2013-03-21 15:08:05 PDT
Created attachment 727914 [details] [diff] [review]
patch passing mozmill test

This patch is passing the test-opening-messages.js::test_open_message_in_new_window test. See https://tbpl.mozilla.org/?tree=Thunderbird-Try&rev=3f63d2a74541

The only change is in messageWindow.css the margin-top/padding-top 0px.
Comment 9 Mike Conley (:mconley) 2013-03-23 12:22:46 PDT
Comment on attachment 727914 [details] [diff] [review]
patch passing mozmill test

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

Code looks good to me. Thanks Paenglab!
Comment 10 Mark Banner (:standard8, limited time in Dec) 2013-03-26 07:43:35 PDT
https://hg.mozilla.org/comm-central/rev/cc1b1733dae2

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