Closed Bug 488118 Opened 12 years ago Closed 12 years ago

Cannot see selected message tag colors in message list (thread) pane

Categories

(Thunderbird :: Folder and Message Lists, defect)

All
Linux
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
Thunderbird 3.0b3

People

(Reporter: ojab, Assigned: clarkbw)

References

Details

(Keywords: regression)

Attachments

(3 files, 1 obsolete file)

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.2a1pre) Gecko/20090413 Minefield/3.6a1pre
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.2a1pre) Gecko/20090413 Lightning/1.0pre Shredder/3.1a1pre

After https://bugzilla.mozilla.org/show_bug.cgi?id=483761 was fixed it's impossible to see if selected message tagged or not. Selection in message pane is always blue (depends on theme, I think, but one color anyway). Previously selection color was dependent on message tags.

In recent builds I should deselect message to see if I should set tag or not, rather annoying.

Reproducible: Always
Without having looked very closely, I'd expect that to be the result of adding 

#threadTree treechildren::-moz-tree-row(selected) {
  background-color: Highlight;
}

which I'm not entirely sure about anyway: toolkit makes (selected) -moz-cellhighlight and (selected, focus) Highlight, so I'd expect this to have regressed being able to tell the difference between those two states, along with probably being more specific than the rule a tag adds (wherever _that_ happens, dunno offhand).
Blocks: 483761
hmm, that's not good.  Yeah, that might be overriding the tagColors.css.  Not sure why I didn't see that when testing.

http://mxr.mozilla.org/comm-central/source/mail/themes/gnomestripe/mail/tagColors.css
Ah, double overriding: a rule in a sheet overrides a rule imported by that sheet, plus |#foo whatever| is (expensively) more specific even if it hadn't already won in the cascade.
Duplicate of this bug: 488466
Status: UNCONFIRMED → NEW
Ever confirmed: true
Keywords: regression
Version: unspecified → Trunk
I've gone with the approach of making the tagColors.css more specific as other methods seem insane and AFAIK the tagColors doesn't need to apply to anything but the #threadPane.

I prefixed the tagColors treechildren selector so it looks like this:

tree#threadTree treechildren

This allows the existing selector to remain as is and tags colors continue to work.  Also changed the Highlight to -moz-cellhighlight and applied it to both (selected), and (selected, focus) rules.  The (selected) rule means the cell is still highlighted but I think it looks a lot better than when it turns into the -moz-oddtreerow color and blends into the background.

Let me know what you think of this approach.
Assignee: nobody → clarkbw
Status: NEW → ASSIGNED
Summary: Cannot see selected message tag in message pane → Cannot see selected message tag colors in message list (thread) pane
Don't know about approach, but with this patch I have gray selection instead of blue. Absolutely looks like inactive selection, but with white font color (as it should be).
just using "!important;" for the background colors seems to work.

Also, per comment 1, in mailWindow.css it should be two different rules - on for selected, and one for selected + focus. 
The selectors can be more specific: #threadTree > treechildren::..... instead of #threadTree treechildren::....
(In reply to comment #7)
> just using "!important;" for the background colors seems to work.

Ok, I can do that instead.

> Also, per comment 1, in mailWindow.css it should be two different rules - on
> for selected, and one for selected + focus.

This I wasn't sure about because of the way we indicate selected but not focused.  Most times the selected, !focused uses the background color change to indicate that.  However that selection background color is the same as the -moz-oddtreerow color and so you end up with either no indication of selection on odd rows or three rows of the same background color.

Right now it changes the font color automatically through toolkit css rules such that we keep the highlight background color for selected, !focus and selected, focus but the font color changes.

> The selectors can be more specific: #threadTree > treechildren::..... instead
> of #threadTree treechildren::....

These didn't look so good to me. The best I could do for a specific selector would be something like this:

#threadTree > .tree-stack > .tree-rows > .tree-bodybox > treechildren

Alternately it could also look like this (using elements instead of classes):

#threadTree > stack > treerows > hbox > treechildren

I know it's better performance but it's also pretty big.  Let me know what you think is best.
I think I'd prefer the element approach.
I don't see this in Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.1b3pre) Gecko/20090227 Lightning/1.0pre Shredder/3.0b2

Fixed?
(In reply to comment #8)
> > The selectors can be more specific: #threadTree > treechildren::..... instead
> > of #threadTree treechildren::....
> 
> These didn't look so good to me. The best I could do for a specific selector
> would be something like this:
> 
> #threadTree > .tree-stack > .tree-rows > .tree-bodybox > treechildren
> 
> Alternately it could also look like this (using elements instead of classes):
> 
> #threadTree > stack > treerows > hbox > treechildren

I was completely wrong about this and your, much simpler approach, works.  I looked in the DOM Inspector and my element selector should have worked but it appears even the toolkit css is using tree > treechildren directly.

patch coming up with your simpler selector and the !important flag on tagColors
(In reply to comment #10)
> I don't see this in Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.1b3pre)
> Gecko/20090227 Lightning/1.0pre Shredder/3.0b2
> 
> Fixed?

No, that's a build from before the regression happened.
here's the patch with all the recommended fixes.  Magnus, can you review this for me?

I'm going to upload a screenshot comparison of the selected, focus differences as well so you can see what it looks like.  I think we've picked the best option keeping focus and selected the same background colour.
Attachment #372940 - Attachment is obsolete: true
Attachment #374278 - Flags: review?(mkmelin+mozilla)
Comment on attachment 374278 [details] [diff] [review]
using the #threadTree > treechildren selector and the !important flag for tagColors

Looks good Bryan! r=mkmelin
Attachment #374278 - Flags: review?(mkmelin+mozilla) → review+
changeset:   2464:eb7247133c36
http://hg.mozilla.org/comm-central/rev/eb7247133c36

->FIXED
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 3.0b3
hmm.. Active selection color still gray instead of blue (without tags) after this patch. Is this appropriate?
Strange.  That makes me wonder what theme you're using and if a different theme doesn't have this issue.
I'm seeing focused selection color in unfocused selection in the threadpane in 20090516 -- the kind of color that mixes badly with tag-colored text.
You need to log in before you can comment on or make changes to this bug.