Closed
Bug 954557
Opened 10 years ago
Closed 10 years ago
Some minor conversation CSS improvements
Categories
(Instantbird Graveyard :: Conversation, defect)
Tracking
(Not tracked)
RESOLVED
FIXED
1.2
People
(Reporter: aleth, Assigned: aleth)
Details
Attachments
(1 file, 3 obsolete files)
2.77 KB,
patch
|
florian
:
review+
|
Details | Diff | Splinter Review |
*** Original post on bio 1124 at 2011-10-30 14:50:00 UTC *** *** Due to BzAPI limitations, the initial description is in comment 1 ***
Assignee | ||
Comment 1•10 years ago
|
||
*** Original post on bio 1124 as attmnt 955 at 2011-10-30 14:50:00 UTC *** Very minor ;) As discussed on IRC.
Attachment #8352697 -
Flags: review?(florian)
Comment 2•10 years ago
|
||
Comment on attachment 8352697 [details] [diff] [review] Patch *** Original change on bio 1124 attmnt 955 at 2011-10-30 16:04:25 UTC *** >--- conversation.xml 2011-10-28 21:32:53.000000000 +0200 >+++ conversation.xml 2011-10-28 02:13:31.000000000 +0200 > <xul:browser anonid="browser" type="content-conversation" flex="1" >- xbl:inherits="tooltip=contenttooltip,contextmenu=contentcontextmenu,autoscrollpopup"/> >+ xbl:inherits="tooltip=contenttooltip,contextmenu=contentcontextmenu,autoscrollpopup,chat"/> Why is the chat attribute needed on this tag? >--- conversation.css 2011-10-30 15:45:05.000000000 +0100 >+++ conversation.css 2011-10-30 15:45:11.000000000 +0100 >@@ -269,20 +269,20 @@ > %endif > } > >-conversation[chat="true"] .conv-messages { >+.conv-messages[chat] { > %ifndef XP_MACOSX > border-right: solid 1px GrayText; > %endif > } While you are cleaning up/optimizing, you could as well move the ifdef out so that the selector only exists in the file when there's going to be a rule for it ;).
Attachment #8352697 -
Flags: review?(florian) → review-
Assignee | ||
Comment 3•10 years ago
|
||
*** Original post on bio 1124 as attmnt 956 at 2011-10-30 16:14:00 UTC *** Corrected patch. Also modified the Mac-only section for consistency, but couldn't test this.
Attachment #8352698 -
Flags: review?(florian)
Assignee | ||
Comment 4•10 years ago
|
||
Comment on attachment 8352697 [details] [diff] [review] Patch *** Original change on bio 1124 attmnt 955 at 2011-10-30 16:14:03 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8352697 -
Attachment is obsolete: true
Assignee | ||
Updated•10 years ago
|
Assignee: nobody → aleth
Status: NEW → ASSIGNED
Assignee | ||
Comment 5•10 years ago
|
||
*** Original post on bio 1124 as attmnt 957 at 2011-10-30 16:20:00 UTC *** Added missing %endif.
Attachment #8352699 -
Flags: review?(florian)
Assignee | ||
Comment 6•10 years ago
|
||
Comment on attachment 8352698 [details] [diff] [review] Patch *** Original change on bio 1124 attmnt 956 at 2011-10-30 16:20:59 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8352698 -
Attachment is obsolete: true
Attachment #8352698 -
Flags: review?(florian)
Assignee | ||
Comment 7•10 years ago
|
||
*** Original post on bio 1124 as attmnt 958 at 2011-10-30 16:27:00 UTC *** This is getting embarrassing...
Attachment #8352700 -
Flags: review?(florian)
Assignee | ||
Comment 8•10 years ago
|
||
Comment on attachment 8352699 [details] [diff] [review] Patch *** Original change on bio 1124 attmnt 957 at 2011-10-30 16:27:22 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8352699 -
Attachment is obsolete: true
Attachment #8352699 -
Flags: review?(florian)
Comment 9•10 years ago
|
||
Comment on attachment 8352700 [details] [diff] [review] Patch *** Original change on bio 1124 attmnt 958 at 2011-10-30 16:40:15 UTC *** Looks good :). I'll try this on Mac before pushing the changes.
Attachment #8352700 -
Flags: review?(florian) → review+
Comment 10•10 years ago
|
||
*** Original post on bio 1124 at 2011-11-09 23:14:52 UTC *** Comment on attachment 8352700 [details] [diff] [review] (bio-attmnt 958) Patch >--- conversation.css 2011-10-30 15:45:35.000000000 +0100 >+++ conversation.css 2011-10-30 17:18:21.000000000 +0100 >@@ -314,7 +314,7 @@ > border-style:none; I've just noticed that a space was missing here between : and none. I've added it (even though it's not directly related to your performance improvements, but as we are cleaning up... :)). >-conversation:not([chat="true"]) .conv-chat { >+.conv-chat:not([chat]) { > display: none; > } The good place for this rule is in instantbird/content/instantbird.css rather than this theme file. I've moved it during my testing. Just for information: we usually put the rules that decide what will be shown (typically -moz-binding and display: none rules) in content stylesheets, and rules that decide how it will be shown in themes stylesheets. Seems to work well, thanks for cleaning this up!
Comment 11•10 years ago
|
||
*** Original post on bio 1124 at 2011-11-10 00:52:26 UTC *** Checked in as http://hg.instantbird.org/instantbird/rev/8430992e1544 Thanks! Sorry it took so long for us to get around to this!
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → 1.2
You need to log in
before you can comment on or make changes to this bug.
Description
•