Closed
Bug 1498908
Opened 6 years ago
Closed 6 years ago
Folders with unread messages no longer distinguishable/bold in folder pane after upgrading to Thunderbird 63.0b1
Categories
(Thunderbird :: Mail Window Front End, defect)
Thunderbird
Mail Window Front End
Tracking
(thunderbird63 wontfix, thunderbird64 fixed)
RESOLVED
FIXED
Thunderbird 64.0
People
(Reporter: Gijs, Assigned: Paenglab)
References
Details
(Keywords: regression)
Attachments
(1 file, 3 obsolete files)
8.79 KB,
patch
|
mkmelin
:
review+
jorgk-bmo
:
approval-comm-beta+
|
Details | Diff | Splinter Review |
Running 63.0b1 (64-bit) on macOS 10.13. The font in the folder pane has changed as part of the upgrade. Perhaps related to this, perhaps not, the folders with unread messages are no longer bold, and as a result it isn't possible for me to see at a glance which folders have unread messages and which don't. This seems related to bug 1498751 if this has to do with e.g. the right atoms being listed for the row in the treeview, which seems likely.
Reporter | ||
Comment 1•6 years ago
|
||
Hm, though bug 1498751 is about the messages list instead of the folder list, so they're probably not directly the same bug.
See Also: 1498751 →
Reporter | ||
Comment 2•6 years ago
|
||
Hm, for a folder with unread messages I still see: "folderNameCol serverType-imap specialFolder-none biffState-NoMail isSecure-true hasUnreadMessages-true noSelect-false imapShared-false" when asking for getRowProperties(<row index that has unread messages>).
Reporter | ||
Comment 3•6 years ago
|
||
If I change: treechildren::-moz-tree-cell-text(hasUnreadMessages-true) { font-weight: bold; } to treechildren::-moz-tree-cell-text(hasUnreadMessages-true) { font-weight: bold !important; } in folderPane.css then the correct behavior returns (though the font is still odd). Richard, do you know why this may have regressed and/or why we need !important now? It's not easy to figure out with the inspector because they're all pseudo-elements...
Assignee | ||
Comment 5•6 years ago
|
||
Yes, the font change was in bug 1461026 and surely the cause of this issue. Gijs, would you be able to review a patch? We are very limited with Mac developers.
Flags: needinfo?(richard.marti)
Reporter | ||
Comment 6•6 years ago
|
||
(In reply to Richard Marti (:Paenglab) from comment #5) > Yes, the font change was in bug 1461026 and surely the cause of this issue. > > Gijs, would you be able to review a patch? We are very limited with Mac > developers. Sure - who's writing it? :-)
Blocks: 1461026
Updated•6 years ago
|
status-thunderbird63:
--- → affected
Keywords: regression
Assignee | ||
Comment 7•6 years ago
|
||
(In reply to :Gijs (he/him) from comment #6) > > Sure - who's writing it? :-) Naturally me. :-) I'm building now.
Assignee | ||
Comment 8•6 years ago
|
||
I increased the specificity to beat the font-weight rule for the Yosemite+ folderPane.
Assignee: nobody → richard.marti
Status: NEW → ASSIGNED
Attachment #9017011 -
Flags: review?(gijskruitbosch+bugs)
Reporter | ||
Comment 9•6 years ago
|
||
Comment on attachment 9017011 [details] [diff] [review] 1498908-unreadSelector.patch Review of attachment 9017011 [details] [diff] [review]: ----------------------------------------------------------------- So this isn't enough to fix the changes from the regressing bug. In particular, the server headers are also no longer blue-text. There might be other issues, I'd have to doublecheck the other rules in folderPane more carefully. I don't really understand why these rules live in mailWindow1.css instead of in the folderPane css code, and why they have the #id selector portion. Can we move them to the folderPane code and reduce their specificity, instead of copying rules from folderPane to mailWindow1.css and increasing their specificity? Sorry if this is a dumb question - I'm not familiar with Thunderbird's architecture.
Attachment #9017011 -
Flags: review?(gijskruitbosch+bugs) → review-
Assignee | ||
Comment 10•6 years ago
|
||
Hmm, here it works. Do you probably use the dark theme? This patch should fix this with adding !important because we need now to overrule the dark theme rules too. This is needed also on Linux and Windows. folderPane.css is not only loaded in the folderPane but also in other locations like the message search window or the menulist with type="folder". I only moved the (imapdeleted) to folderPane.css which also makes sense in the message search window.
Attachment #9017011 -
Attachment is obsolete: true
Attachment #9017032 -
Flags: review?(gijskruitbosch+bugs)
Assignee | ||
Comment 11•6 years ago
|
||
Oops, sorry the wrong patch.
Attachment #9017032 -
Attachment is obsolete: true
Attachment #9017032 -
Flags: review?(gijskruitbosch+bugs)
Attachment #9017033 -
Flags: review?(gijskruitbosch+bugs)
Reporter | ||
Comment 12•6 years ago
|
||
Comment on attachment 9017033 [details] [diff] [review] 1498908-unreadSelector.patch Review of attachment 9017033 [details] [diff] [review]: ----------------------------------------------------------------- I'll believe that this fixes the issue, but it's hard for me to verify it doesn't introduce new, other issues - !important means we now end up overriding the color + font-weight everywhere, even if there were other rules in place for those that really *should* override the ones in folderPane.css (unlike the one in mailWindow1.css, which *shouldn't*). So f+ from me on fixing the problem, but I think you should get an r+ from a mail peer who understands the codebase better and knows for sure if there are other places that this would affect. In particular, I worry about the effect this will have on highlighted/selected rows / text, also in menulist dropdowns. More generally, the !important makes me sad, but based on what you said I'm not sure there are easy, better alternatives. As an alternative suggestion: could we use a different selector for the folder pane in mailWindow1.css that wouldn't increase the specificity so much, and/or are there non-ID (maybe class?) selectors we could use in folderPane.css to increase specificity that would apply to all the different usage locations? Or are there none really available?
Attachment #9017033 -
Flags: review?(gijskruitbosch+bugs) → feedback+
Reporter | ||
Comment 13•6 years ago
|
||
(In reply to Richard Marti (:Paenglab) from comment #10) > Created attachment 9017032 [details] [diff] [review] > themeTrees.patch > > Hmm, here it works. To be clear, I didn't test the patch (I don't have a setup for that). I was going by code inspection, and the fact that the colour of the server rows is gone in my "production" beta copy that I use for my day-to-day email. Your original patch "only" added a rule for font-weight, but the patch in bug 1461026 increased specificity for the rules governing text color (ie css color: property), too, so I would expect the same issues with the server row and this rule: https://dxr.mozilla.org/comm-central/source/comm/mail/themes/osx/mail/folderPane.css#338 . Am I missing something? > Do you probably use the dark theme? Nope.
Assignee | ||
Comment 14•6 years ago
|
||
To the previous patch I also tidied the new message colours to use always the same colour also when the row is actually selected. I could also use #folderTree instead of the !important but then the highlighting etc. in for example the folder selector in the smart filter dialog would no more woark as it does now. I tested this and saw no problems with the !important.
Attachment #9017033 -
Attachment is obsolete: true
Attachment #9017093 -
Flags: review?(mkmelin+mozilla)
Comment 15•6 years ago
|
||
Comment on attachment 9017093 [details] [diff] [review] 1498908-unreadSelector.patch Review of attachment 9017093 [details] [diff] [review]: ----------------------------------------------------------------- Seems to work. r=mkmelin
Attachment #9017093 -
Flags: review?(mkmelin+mozilla) → review+
Assignee | ||
Updated•6 years ago
|
Keywords: checkin-needed
Comment 16•6 years ago
|
||
Comment on attachment 9017093 [details] [diff] [review] 1498908-unreadSelector.patch I guess you want to fix TB 63b1. I haven't tried whether the patch applies.
Attachment #9017093 -
Flags: approval-comm-beta+
Comment 17•6 years ago
|
||
TB 63.0b2: https://hg.mozilla.org/releases/comm-beta/rev/e9f0f4d5d705 Trunk will land in a minute. Gijs, can you pick a build from this push, please: https://treeherder.mozilla.org/#/jobs?repo=comm-beta&revision=e9f0f4d5d7053a5e0cad397e73b1989199965c90
status-thunderbird64:
--- → fixed
Target Milestone: --- → Thunderbird 64.0
Comment 18•6 years ago
|
||
Pushed by mozilla@jorgk.com: https://hg.mozilla.org/comm-central/rev/3e5b48477fb4 Increase the specificity of the unread messages treechildren selectors. r=mkmelin
Reporter | ||
Comment 19•6 years ago
|
||
(In reply to Jorg K (GMT+2) from comment #17) > TB 63.0b2: > https://hg.mozilla.org/releases/comm-beta/rev/e9f0f4d5d705 > > Trunk will land in a minute. Gijs, can you pick a build from this push, > please: > https://treeherder.mozilla.org/#/jobs?repo=comm- > beta&revision=e9f0f4d5d7053a5e0cad397e73b1989199965c90 LGTM, thank you!
Reporter | ||
Comment 20•6 years ago
|
||
Is there a timescale for the release of a beta with the fix? I looked at https://wiki.mozilla.org/Thunderbird:Home#Releases but that didn't really help me...
Comment 21•6 years ago
|
||
(In reply to :Gijs (he/him) from comment #20) > Is there a timescale for the release of a beta with the fix? I looked at > https://wiki.mozilla.org/Thunderbird:Home#Releases but that didn't really > help me... If all goes well, next week. If does not go well, which is frequent, then not possible to predict.
Comment 23•6 years ago
|
||
I reported 1499431. It's not just lack of bold, the new folder tree font weight is off relative to native UI. Needs to be slightly heavier overall. See screenshot in referenced bug.
Comment 24•6 years ago
|
||
Gijs, thanks for verifying, but we're not shipping a TB 63 beta 2.
You need to log in
before you can comment on or make changes to this bug.
Description
•