Closed
Bug 1003295
Opened 10 years ago
Closed 10 years ago
Win8: Text color on inactive tabs makes text very hard to read (FX bug 940393)
Categories
(Thunderbird :: Theme, defect)
Tracking
(thunderbird31 fixed)
RESOLVED
FIXED
Thunderbird 32.0
Tracking | Status | |
---|---|---|
thunderbird31 | --- | fixed |
People
(Reporter: Paenglab, Assigned: Paenglab)
Details
Attachments
(1 file, 3 obsolete files)
13.17 KB,
patch
|
Paenglab
:
review+
Paenglab
:
ui-review+
standard8
:
approval-comm-aurora+
|
Details | Diff | Splinter Review |
Win( uses always black as titlebar text color. This makes it hard to read inactive tabs text (see attachment 8334525 [details]). Fx fixed this with bug 940393 and bug 988191.
Assignee | ||
Comment 1•10 years ago
|
||
The JS part is a copy of bug 940393 and bug 988191 (no change was needed). So I would say this code itself isn't needed to review but the position where it is inserted. Philipp, I added you for the Calendar CSS code.
Assignee: nobody → richard.marti
Status: NEW → ASSIGNED
Attachment #8414632 -
Flags: ui-review?(mconley)
Attachment #8414632 -
Flags: review?(philipp)
Attachment #8414632 -
Flags: review?(mconley)
Assignee | ||
Comment 2•10 years ago
|
||
Not all Lighning icons where correctly shown on tabbar-toolbar in customize mode.
Attachment #8414632 -
Attachment is obsolete: true
Attachment #8414632 -
Flags: ui-review?(mconley)
Attachment #8414632 -
Flags: review?(philipp)
Attachment #8414632 -
Flags: review?(mconley)
Attachment #8414743 -
Flags: ui-review?(mconley)
Attachment #8414743 -
Flags: review?(philipp)
Attachment #8414743 -
Flags: review?(mconley)
Updated•10 years ago
|
Attachment #8414743 -
Flags: review?(philipp) → review+
Assignee | ||
Comment 3•10 years ago
|
||
Updated to follow also bug 1007229. Mike, could you only look if the position of the new JS code in msgMail3PaneWindow.js is at the correct position? Where is no code change (except the added comment at the beginning) to the FX code. Josiah, is it okay I added you for the r and ui-r of the CSS code?
Attachment #8414743 -
Attachment is obsolete: true
Attachment #8414743 -
Flags: ui-review?(mconley)
Attachment #8414743 -
Flags: review?(mconley)
Attachment #8420575 -
Flags: ui-review?(josiah)
Attachment #8420575 -
Flags: review?(mconley)
Attachment #8420575 -
Flags: review?(josiah)
Comment 4•10 years ago
|
||
Comment on attachment 8420575 [details] [diff] [review] Win8-tabReadability.patch Review of attachment 8420575 [details] [diff] [review]: ----------------------------------------------------------------- The code looks okay, but I haven't looked at the product myself, hopefully Mike can. I'm very busy for the next couple of days, so if he hasn't ui-reviewed by Wednesday then feel free to re-assign the flag.
Attachment #8420575 -
Flags: ui-review?(josiah)
Attachment #8420575 -
Flags: review?(josiah)
Attachment #8420575 -
Flags: review+
Assignee | ||
Comment 5•10 years ago
|
||
Try build for easier ui-r: https://tbpl.mozilla.org/?tree=Thunderbird-Try&rev=b39b2b01c07f
Assignee | ||
Comment 6•10 years ago
|
||
Comment on attachment 8420575 [details] [diff] [review] Win8-tabReadability.patch Unfortunately Mike had no time. Josiah, if you have time, could you look at it? The try build should still work.
Attachment #8420575 -
Flags: ui-review?(josiah)
Comment 7•10 years ago
|
||
Comment on attachment 8420575 [details] [diff] [review] Win8-tabReadability.patch Review of attachment 8420575 [details] [diff] [review]: ----------------------------------------------------------------- Looks okay to me.
Attachment #8420575 -
Flags: ui-review?(josiah) → ui-review+
Comment 8•10 years ago
|
||
Comment on attachment 8420575 [details] [diff] [review] Win8-tabReadability.patch Review of attachment 8420575 [details] [diff] [review]: ----------------------------------------------------------------- This looks fine. I filed bug 1013654 to get some real platform support on the menu / tabs toolbar colours, but until we get something there, I guess forking / cloning the Windows8WindowFrameColor.jsm isn't so bad. Maybe file a bug that depends on bug 1013654 to remote the JSM when platform support is available. You might want Fallen's r+ on the Lightning bits. ::: mail/base/content/msgMail3PaneWindow.js @@ +400,5 @@ > UpdateMailPaneConfig(false); > document.loadBindingDocument('chrome://global/content/bindings/textbox.xml'); > + > +#ifdef XP_WIN > + // On Win8 set an attribute when the window frame color is to dark for black text. Nit: s/to/too ::: mail/themes/windows/mail/tabmail-aero.css @@ +229,5 @@ > + } > + > + #messengerWindow[darkwindowframe="true"] .tabmail-arrowscrollbox > > + .scrollbutton-up:not(:-moz-lwtheme):not(:-moz-window-inactive), > + #messengerWindow[darkwindowframe="true"] .tabmail-arrowscrollbox Have we always broken up lines like this? I find it so hard to read. :/ I'd honestly prefer to have the rules on one line, and not worry about the 80 char limit here. Feel free to do that.
Attachment #8420575 -
Flags: review?(mconley) → review+
Assignee | ||
Comment 9•10 years ago
|
||
(In reply to Mike Conley (:mconley) from comment #8) > Comment on attachment 8420575 [details] [diff] [review] > Win8-tabReadability.patch > > Review of attachment 8420575 [details] [diff] [review]: > ----------------------------------------------------------------- > > ::: mail/base/content/msgMail3PaneWindow.js > > + > > +#ifdef XP_WIN > > + // On Win8 set an attribute when the window frame color is to dark for black text. > > Nit: s/to/too Done > ::: mail/themes/windows/mail/tabmail-aero.css > @@ +229,5 @@ > > + } > > + > > + #messengerWindow[darkwindowframe="true"] .tabmail-arrowscrollbox > > > + .scrollbutton-up:not(:-moz-lwtheme):not(:-moz-window-inactive), > > + #messengerWindow[darkwindowframe="true"] .tabmail-arrowscrollbox > > Have we always broken up lines like this? I find it so hard to read. :/ I'd > honestly prefer to have the rules on one line, and not worry about the 80 > char limit here. Feel free to do that. Yes, Blake was very strict on this, I leave it here as it is.
Attachment #8420575 -
Attachment is obsolete: true
Attachment #8426062 -
Flags: ui-review+
Attachment #8426062 -
Flags: review+
Assignee | ||
Updated•10 years ago
|
Keywords: checkin-needed
Assignee | ||
Comment 10•10 years ago
|
||
Comment on attachment 8426062 [details] [diff] [review] patch for check-in [Approval Request Comment] User impact if declined: On dark wndow frames the user could have problems to read the tab titiles Testing completed (on c-c, etc.): in c-c, the JS code is in m-c since FX 31 Risk to taking this patch (and alternatives if risky): low, code is already tested i FX
Attachment #8426062 -
Flags: approval-comm-aurora?
Comment 11•10 years ago
|
||
https://hg.mozilla.org/comm-central/rev/007931222e16
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 32.0
Updated•10 years ago
|
Attachment #8426062 -
Flags: approval-comm-aurora? → approval-comm-aurora+
Comment 12•10 years ago
|
||
https://hg.mozilla.org/releases/comm-aurora/rev/08b288b58838
status-thunderbird31:
--- → fixed
Updated•9 years ago
|
Flags: in-testsuite-
You need to log in
before you can comment on or make changes to this bug.
Description
•