Closed Bug 1145974 Opened 5 years ago Closed 5 years ago

Move more styles to shared addressbook.css

Categories

(Thunderbird :: Theme, defect)

defect
Not set

Tracking

(thunderbird40 fixed)

RESOLVED FIXED
Thunderbird 40.0
Tracking Status
thunderbird40 --- fixed

People

(Reporter: Paenglab, Assigned: Paenglab)

Details

Attachments

(1 file, 2 obsolete files)

No description provided.
Attached patch sharedAB.patch (obsolete) β€” β€” Splinter Review
I've shared as much as I saw could be done.
Assignee: nobody → richard.marti
Status: NEW → ASSIGNED
Attachment #8581132 - Flags: review?(josiah)
Summary: Move more styles to shared addressnook.css → Move more styles to shared addressbook.css
Attached patch sharedAB.patch (obsolete) β€” β€” Splinter Review
Fixed the commit message.
Attachment #8581132 - Attachment is obsolete: true
Attachment #8581132 - Flags: review?(josiah)
Attachment #8581257 - Flags: review?(josiah)
Comment on attachment 8581257 [details] [diff] [review]
sharedAB.patch

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

Looks good, I just had a few comments/questions.
Attachment #8581257 - Flags: review?(josiah) → review+
Comment on attachment 8581257 [details] [diff] [review]
sharedAB.patch

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

... Which all disappeared ... *Sigh*

::: mail/themes/osx/mail/addrbook/addressbook.css
@@ -377,5 @@
>  
>  #abContent {
>    color: -moz-DialogText;
> -  text-shadow: none;
> -  background-color: -moz-Dialog;

Didn't we decide that we would only share if all selector rules matched?

@@ +554,1 @@
>    min-height: 19px; /* aqua size for small buttons */

Ditto.

::: mail/themes/windows/mail/addrbook/addressbook-aero.css
@@ -502,5 @@
> -  list-style-image: url("chrome://messenger/skin/addressbook/icons/ablist.png");
> -}
> -
> -treechildren::-moz-tree-cell-text(GeneratedName) {
> -  -moz-padding-start: 0 !important;

Ditto.

::: mail/themes/windows/mail/addrbook/addressbook.css
@@ +190,1 @@
>    border-top: 1px solid ThreeDShadow;

Ditto.

@@ -294,5 @@
> -  list-style-image: url("chrome://messenger/skin/addressbook/icons/ablist.png");
> -}
> -
> -treechildren::-moz-tree-cell-text(GeneratedName) {
> -  -moz-padding-start: 0 !important;

I assume you made sure this works now that the !important is gone?
Attached patch sharedAB.patch β€” β€” Splinter Review
Yes, you're right. This patch shares now only full matches.
Attachment #8581257 - Attachment is obsolete: true
Attachment #8590449 - Flags: review?(josiah)
(In reply to Josiah Bruner [:JosiahOne] (needinfo! - Won't respond to CC)) from comment #4)
> Comment on attachment 8581257 [details] [diff] [review]
> sharedAB.patch
> 
> Review of attachment 8581257 [details] [diff] [review]:
> -----------------------------------------------------------------
> @@ -294,5 @@
> > -  list-style-image: url("chrome://messenger/skin/addressbook/icons/ablist.png");
> > -}
> > -
> > -treechildren::-moz-tree-cell-text(GeneratedName) {
> > -  -moz-padding-start: 0 !important;
> 
> I assume you made sure this works now that the !important is gone?

Yes, I haven't seen why this !important was set or is needed.
Attachment #8590449 - Flags: review?(josiah) → review+
Keywords: checkin-needed
https://hg.mozilla.org/comm-central/rev/423864cb6cba
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 40.0
You need to log in before you can comment on or make changes to this bug.