Closed
Bug 1423453
Opened 7 years ago
Closed 7 years ago
Fix splitter and border styling on Windows
Categories
(Toolkit :: Themes, defect)
Tracking
()
VERIFIED
FIXED
mozilla59
Tracking | Status | |
---|---|---|
firefox-esr52 | --- | unaffected |
firefox57 | --- | unaffected |
firefox58 | --- | unaffected |
firefox59 | --- | verified |
People
(Reporter: magicp.jp, Assigned: Paenglab)
References
Details
Attachments
(6 files, 9 obsolete files)
Steps to reproduce:
1. Launch the latest Nightly on Windows
2. Go to about:home
3. Show Bookmarks sidebar
4. Open Storage Inspector
5. Go to about:preferences > General > Home page
6. Click "Use Bookmark..." button
Actual Results:
In step 2, sidebar separator is dark.
In step 4, separator styling is incorrect.
In step 5, tree has no border-bottom and border-right.
Expected Results:
Previous styling
Regression range:
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=32ded91397e63d638bcaec92c82978654030f9fc&tochange=e6af6f8ae63e29794e0e6da5a60a576ebae621ee
Blocks: 1417200
Has Regression Range: --- → yes
Has STR: --- → yes
status-firefox57:
--- → unaffected
status-firefox58:
--- → unaffected
status-firefox-esr52:
--- → unaffected
separator -> splitter
Summary: Fix separator and border styling on Windows → Fix splitter and border styling on Windows
Comment 3•7 years ago
|
||
I don't have a Windows dev env and apparently this is Windows only (that's why I missed it in bug 1417200 :/).
If someone can look into it soon, that'd be nice, otherwise I can take care of it after the All-Hands.
Comment 4•7 years ago
|
||
This impacts the storage inspector tool in DevTools pretty negatively, so I;d really love to have this fixed in 59. Looks like the merge is sometimes in January, so that should leave us enough time.
Emilio: is this a bug that needs to be fixed in the code that landed with bug 1417200, or are there things that need to be done in devtools CSS to apply the right styling again?
Comment 5•7 years ago
|
||
(In reply to Patrick Brosset <:pbro> from comment #4)
> This impacts the storage inspector tool in DevTools pretty negatively, so
> I;d really love to have this fixed in 59. Looks like the merge is sometimes
> in January, so that should leave us enough time.
> Emilio: is this a bug that needs to be fixed in the code that landed with
> bug 1417200, or are there things that need to be done in devtools CSS to
> apply the right styling again?
Ideally devtools would stop using -moz-border-*-colors, yeah.
Comment 6•7 years ago
|
||
For the record, devtools is just using XUL splitter elements that happen to use -moz-border-*-colors on windows. https://searchfox.org/mozilla-central/source/toolkit/themes/windows/global/splitter.css
Assignee | ||
Comment 7•7 years ago
|
||
Dão, what do you think about this? It removes the -moz-border-*-colors rules where the -moz-appearance isn't 'none' and not used, like in your button.css cleanups. On the other places I used normal border-color rules.
Assignee: nobody → richard.marti
Status: NEW → ASSIGNED
Attachment #8935901 -
Flags: review?(dao+bmo)
Comment 8•7 years ago
|
||
Comment on attachment 8935901 [details] [diff] [review]
Bug1423453.patch
>--- a/toolkit/themes/windows/global/listbox.css
>+++ b/toolkit/themes/windows/global/listbox.css
> listbox {
> -moz-appearance: listbox;
> margin: 2px 4px;
>- border: 2px solid;
>- -moz-border-top-colors: ThreeDShadow ThreeDDarkShadow;
>- -moz-border-right-colors: ThreeDHighlight ThreeDLightShadow;
>- -moz-border-bottom-colors: ThreeDHighlight ThreeDLightShadow;
>- -moz-border-left-colors: ThreeDShadow ThreeDDarkShadow;
>+ border: 1px solid ThreeDShadow;
>+ border-inline-end-color: ThreeDHighlight;
>+ border-bottom-color: ThreeDHighlight;
Does -moz-appearance: listbox; not take care of the border on Windows?
>--- a/toolkit/themes/windows/global/popup.css
>+++ b/toolkit/themes/windows/global/popup.css
> menupopup,
> panel {
>- border: 3px solid transparent;
>- -moz-border-top-colors : ThreeDLightShadow ThreeDHighlight ThreeDFace;
>- -moz-border-left-colors : ThreeDLightShadow ThreeDHighlight ThreeDFace;
>- -moz-border-right-colors : ThreeDDarkShadow ThreeDShadow ThreeDFace;
>- -moz-border-bottom-colors: ThreeDDarkShadow ThreeDShadow ThreeDFace;
>- padding: 0px;
>+ border: 1px solid ThreeDHighlight;
>+ border-inline-end-color: ThreeDShadow;
>+ border-bottom-color: ThreeDShadow;
>+ padding: 0;
> min-width: 1px;
> background: Menu;
> color: MenuText;
> }
How does this end up looking in practice?
Comment 9•7 years ago
|
||
Bug 1417200 also added borders to dropdown menus (in about:preferences, about:addons etc), see attachment.
Will this bug solve also this issue, or it should be tracked separately? Thank you!
Flags: needinfo?(richard.marti)
Assignee | ||
Comment 10•7 years ago
|
||
(In reply to Dão Gottwald [::dao] from comment #8)
> Comment on attachment 8935901 [details] [diff] [review]
> Bug1423453.patch
>
> >--- a/toolkit/themes/windows/global/listbox.css
> >+++ b/toolkit/themes/windows/global/listbox.css
>
> > listbox {
> > -moz-appearance: listbox;
> > margin: 2px 4px;
> >- border: 2px solid;
> >- -moz-border-top-colors: ThreeDShadow ThreeDDarkShadow;
> >- -moz-border-right-colors: ThreeDHighlight ThreeDLightShadow;
> >- -moz-border-bottom-colors: ThreeDHighlight ThreeDLightShadow;
> >- -moz-border-left-colors: ThreeDShadow ThreeDDarkShadow;
> >+ border: 1px solid ThreeDShadow;
> >+ border-inline-end-color: ThreeDHighlight;
> >+ border-bottom-color: ThreeDHighlight;
>
> Does -moz-appearance: listbox; not take care of the border on Windows?
Yes, it does. Removed the border.
Attachment #8935901 -
Attachment is obsolete: true
Attachment #8935901 -
Flags: review?(dao+bmo)
Flags: needinfo?(richard.marti)
Attachment #8936423 -
Flags: review?(dao+bmo)
Assignee | ||
Comment 11•7 years ago
|
||
(In reply to Dão Gottwald [::dao] from comment #8)
> Comment on attachment 8935901 [details] [diff] [review]
> Bug1423453.patch
>
> >--- a/toolkit/themes/windows/global/popup.css
> >+++ b/toolkit/themes/windows/global/popup.css
>
> > menupopup,
> > panel {
> >- border: 3px solid transparent;
> >- -moz-border-top-colors : ThreeDLightShadow ThreeDHighlight ThreeDFace;
> >- -moz-border-left-colors : ThreeDLightShadow ThreeDHighlight ThreeDFace;
> >- -moz-border-right-colors : ThreeDDarkShadow ThreeDShadow ThreeDFace;
> >- -moz-border-bottom-colors: ThreeDDarkShadow ThreeDShadow ThreeDFace;
> >- padding: 0px;
> >+ border: 1px solid ThreeDHighlight;
> >+ border-inline-end-color: ThreeDShadow;
> >+ border-bottom-color: ThreeDShadow;
> >+ padding: 0;
> > min-width: 1px;
> > background: Menu;
> > color: MenuText;
> > }
>
> How does this end up looking in practice?
Screenshot on Win7 Classic with forced -moz-appearance: none;
Assignee | ||
Comment 12•7 years ago
|
||
(In reply to Petruta Rasa [QA] [:petruta] from comment #9)
> Created attachment 8936385 [details]
> dropdown border.png
>
> Bug 1417200 also added borders to dropdown menus (in about:preferences,
> about:addons etc), see attachment.
>
> Will this bug solve also this issue, or it should be tracked separately?
> Thank you!
Yes it fixed this too.
Comment 13•7 years ago
|
||
(In reply to Richard Marti (:Paenglab) from comment #11)
> Created attachment 8936424 [details]
> menupopup with patch applied
>
> (In reply to Dão Gottwald [::dao] from comment #8)
> > Comment on attachment 8935901 [details] [diff] [review]
> > Bug1423453.patch
> >
> > >--- a/toolkit/themes/windows/global/popup.css
> > >+++ b/toolkit/themes/windows/global/popup.css
> >
> > > menupopup,
> > > panel {
> > >- border: 3px solid transparent;
> > >- -moz-border-top-colors : ThreeDLightShadow ThreeDHighlight ThreeDFace;
> > >- -moz-border-left-colors : ThreeDLightShadow ThreeDHighlight ThreeDFace;
> > >- -moz-border-right-colors : ThreeDDarkShadow ThreeDShadow ThreeDFace;
> > >- -moz-border-bottom-colors: ThreeDDarkShadow ThreeDShadow ThreeDFace;
> > >- padding: 0px;
> > >+ border: 1px solid ThreeDHighlight;
> > >+ border-inline-end-color: ThreeDShadow;
> > >+ border-bottom-color: ThreeDShadow;
> > >+ padding: 0;
> > > min-width: 1px;
> > > background: Menu;
> > > color: MenuText;
> > > }
> >
> > How does this end up looking in practice?
>
> Screenshot on Win7 Classic with forced -moz-appearance: none;
In other words
Blocks: 1341048
Comment 14•7 years ago
|
||
(In reply to Dão Gottwald [::dao] from comment #13)
> > > > menupopup,
> > > > panel {
> > > >- border: 3px solid transparent;
> > > >- -moz-border-top-colors : ThreeDLightShadow ThreeDHighlight ThreeDFace;
> > > >- -moz-border-left-colors : ThreeDLightShadow ThreeDHighlight ThreeDFace;
> > > >- -moz-border-right-colors : ThreeDDarkShadow ThreeDShadow ThreeDFace;
> > > >- -moz-border-bottom-colors: ThreeDDarkShadow ThreeDShadow ThreeDFace;
> > > >- padding: 0px;
> > > >+ border: 1px solid ThreeDHighlight;
> > > >+ border-inline-end-color: ThreeDShadow;
> > > >+ border-bottom-color: ThreeDShadow;
> > > >+ padding: 0;
> > > > min-width: 1px;
> > > > background: Menu;
> > > > color: MenuText;
> > > > }
> > >
> > > How does this end up looking in practice?
> >
> > Screenshot on Win7 Classic with forced -moz-appearance: none;
>
> In other words
Oops, let me finish that comment. Does "forced -moz-appearance: none;" mean that normally this styling wouldn't be used?
Assignee | ||
Comment 15•7 years ago
|
||
The menupopup has the -moz-appearance set here: https://dxr.mozilla.org/mozilla-central/source/toolkit/themes/windows/global/popup.css#22
And panel[type="arrow"] here: https://dxr.mozilla.org/mozilla-central/source/toolkit/themes/windows/global/popup.css#32
But the plain panel has nothing set. The panel[type="autocomplete"] and the panel[type="autocomplete-richlistbox"] have set it to -moz-appearance: none here: https://dxr.mozilla.org/mozilla-central/source/toolkit/themes/windows/global/autocomplete.css#20 but override the border-colors.
Good you ask, I see now that I can change autocomplete.css to border-color too. Updated patch follows this evening.
Assignee | ||
Comment 16•7 years ago
|
||
The -moz-border-*-colors removed in autocomplete.css
Attachment #8936423 -
Attachment is obsolete: true
Attachment #8936423 -
Flags: review?(dao+bmo)
Attachment #8936626 -
Flags: review?(dao+bmo)
Comment 17•7 years ago
|
||
Comment on attachment 8936626 [details] [diff] [review]
Bug1423453.patch
>--- a/browser/themes/windows/browser.css
>+++ b/browser/themes/windows/browser.css
> #ContentSelectDropdown > menupopup {
> background-color: -moz-field;
>- -moz-border-top-colors: GrayText;
>- -moz-border-right-colors: GrayText;
>- -moz-border-bottom-colors: GrayText;
>- -moz-border-left-colors: GrayText;
>+ border-color: GrayText;
> }
Doesn't this use -moz-appearance: menupopup;?
>--- a/toolkit/themes/windows/global/menulist.css
>+++ b/toolkit/themes/windows/global/menulist.css
> menulist {
> -moz-appearance: menulist;
> margin: 2px 4px;
>- border: 2px solid;
>- -moz-border-top-colors: ThreeDShadow ThreeDDarkShadow;
>- -moz-border-right-colors: ThreeDHighlight ThreeDLightShadow;
>- -moz-border-bottom-colors: ThreeDHighlight ThreeDLightShadow;
>- -moz-border-left-colors: ThreeDShadow ThreeDDarkShadow;
> background-color: -moz-Field;
Could also remove the background-color. AFAIK it's unused just like the border.
>--- a/toolkit/themes/windows/global/popup.css
>+++ b/toolkit/themes/windows/global/popup.css
> menupopup,
> panel {
>- border: 3px solid transparent;
>- -moz-border-top-colors : ThreeDLightShadow ThreeDHighlight ThreeDFace;
>- -moz-border-left-colors : ThreeDLightShadow ThreeDHighlight ThreeDFace;
>- -moz-border-right-colors : ThreeDDarkShadow ThreeDShadow ThreeDFace;
>- -moz-border-bottom-colors: ThreeDDarkShadow ThreeDShadow ThreeDFace;
>- padding: 0px;
>+ border: 1px solid ThreeDHighlight;
>+ border-inline-end-color: ThreeDShadow;
>+ border-bottom-color: ThreeDShadow;
>+ padding: 0;
> min-width: 1px;
> background: Menu;
> color: MenuText;
> }
I wonder if we should just set border: 1px solid ThreeDShadow; here just like for panel[type="autocomplete"].
Attachment #8936626 -
Flags: review?(dao+bmo)
Assignee | ||
Comment 18•7 years ago
|
||
(In reply to Dão Gottwald [::dao] from comment #17)
> Comment on attachment 8936626 [details] [diff] [review]
> Bug1423453.patch
>
> >--- a/browser/themes/windows/browser.css
> >+++ b/browser/themes/windows/browser.css
>
> > #ContentSelectDropdown > menupopup {
> > background-color: -moz-field;
> >- -moz-border-top-colors: GrayText;
> >- -moz-border-right-colors: GrayText;
> >- -moz-border-bottom-colors: GrayText;
> >- -moz-border-left-colors: GrayText;
> >+ border-color: GrayText;
> > }
>
> Doesn't this use -moz-appearance: menupopup;?
No, it uses the -moz-appearance: none; from menulist > menupopup. But the background color is already defined in menulist > menupopup
> >--- a/toolkit/themes/windows/global/menulist.css
> >+++ b/toolkit/themes/windows/global/menulist.css
>
> > menulist {
> > -moz-appearance: menulist;
> > margin: 2px 4px;
> >- border: 2px solid;
> >- -moz-border-top-colors: ThreeDShadow ThreeDDarkShadow;
> >- -moz-border-right-colors: ThreeDHighlight ThreeDLightShadow;
> >- -moz-border-bottom-colors: ThreeDHighlight ThreeDLightShadow;
> >- -moz-border-left-colors: ThreeDShadow ThreeDDarkShadow;
> > background-color: -moz-Field;
>
> Could also remove the background-color. AFAIK it's unused just like the
> border.
Yes, done.
> >--- a/toolkit/themes/windows/global/popup.css
> >+++ b/toolkit/themes/windows/global/popup.css
>
> > menupopup,
> > panel {
> >- border: 3px solid transparent;
> >- -moz-border-top-colors : ThreeDLightShadow ThreeDHighlight ThreeDFace;
> >- -moz-border-left-colors : ThreeDLightShadow ThreeDHighlight ThreeDFace;
> >- -moz-border-right-colors : ThreeDDarkShadow ThreeDShadow ThreeDFace;
> >- -moz-border-bottom-colors: ThreeDDarkShadow ThreeDShadow ThreeDFace;
> >- padding: 0px;
> >+ border: 1px solid ThreeDHighlight;
> >+ border-inline-end-color: ThreeDShadow;
> >+ border-bottom-color: ThreeDShadow;
> >+ padding: 0;
> > min-width: 1px;
> > background: Menu;
> > color: MenuText;
> > }
>
> I wonder if we should just set border: 1px solid ThreeDShadow; here just
> like for panel[type="autocomplete"].
Also done and removed the border-color in panel[type="autocomplete"].
Attachment #8936626 -
Attachment is obsolete: true
Attachment #8936878 -
Flags: review?(dao+bmo)
Comment 19•7 years ago
|
||
> > > #ContentSelectDropdown > menupopup {
> > > background-color: -moz-field;
> > >- -moz-border-top-colors: GrayText;
> > >- -moz-border-right-colors: GrayText;
> > >- -moz-border-bottom-colors: GrayText;
> > >- -moz-border-left-colors: GrayText;
> > >+ border-color: GrayText;
> > > }
> >
> > Doesn't this use -moz-appearance: menupopup;?
>
> No, it uses the -moz-appearance: none; from menulist > menupopup. But the
> background color is already defined in menulist > menupopup
These border color choices seem a bit weird. Shouldn't these popups just use ThreeDShadow?
Assignee | ||
Comment 20•7 years ago
|
||
I can't say how it looks as I don't know how to activate it. I only checked it in Inspector. Do you have a STR to show it?
Comment 21•7 years ago
|
||
(In reply to Richard Marti (:Paenglab) from comment #20)
> I can't say how it looks as I don't know how to activate it. I only checked
> it in Inspector. Do you have a STR to show it?
This popup is used for <select> elements in the content area (e.g. on this bugzilla page).
Component: Themes → Microformats
Updated•7 years ago
|
Component: Microformats → Themes
Assignee | ||
Comment 22•7 years ago
|
||
Comparison of the ContentSelectDropdown popup with GrayText border on the left and default -moz-FieldText border on the right.
I think the GrayText fits better to the border of the <select> item.
Assignee | ||
Comment 23•7 years ago
|
||
Sorry, one change wasn't in the previous patch.
Attachment #8936878 -
Attachment is obsolete: true
Attachment #8936878 -
Flags: review?(dao+bmo)
Attachment #8936897 -
Flags: review?(dao+bmo)
Comment 24•7 years ago
|
||
(In reply to Richard Marti (:Paenglab) from comment #22)
> Created attachment 8936892 [details]
> ContentSelectDropdown-popup.png
>
> Comparison of the ContentSelectDropdown popup with GrayText border on the
> left and default -moz-FieldText border on the right.
>
> I think the GrayText fits better to the border of the <select> item.
That's why I said threedshadow. Graytext and -moz-FieldText are text colors, so they don't really make sense here.
Assignee | ||
Comment 25•7 years ago
|
||
You're right, ThreeDShadow is better.
Attachment #8936897 -
Attachment is obsolete: true
Attachment #8936897 -
Flags: review?(dao+bmo)
Attachment #8936915 -
Flags: review?(dao+bmo)
Comment 26•7 years ago
|
||
Comment on attachment 8936915 [details] [diff] [review]
Bug1423453.patch
Can you remove the #ContentSelectDropdown > menupopup rule?
Attachment #8936915 -
Flags: review?(dao+bmo)
Assignee | ||
Comment 27•7 years ago
|
||
(In reply to Dão Gottwald [::dao] from comment #26)
> Comment on attachment 8936915 [details] [diff] [review]
> Bug1423453.patch
>
> Can you remove the #ContentSelectDropdown > menupopup rule?
Done
Attachment #8936915 -
Attachment is obsolete: true
Attachment #8936931 -
Flags: review?(dao+bmo)
Comment 28•7 years ago
|
||
Comment on attachment 8936931 [details] [diff] [review]
Bug1423453.patch
>--- a/toolkit/themes/windows/global/listbox.css
>+++ b/toolkit/themes/windows/global/listbox.css
> listbox {
> -moz-appearance: listbox;
> margin: 2px 4px;
>- border: 2px solid;
>- -moz-border-top-colors: ThreeDShadow ThreeDDarkShadow;
>- -moz-border-right-colors: ThreeDHighlight ThreeDLightShadow;
>- -moz-border-bottom-colors: ThreeDHighlight ThreeDLightShadow;
>- -moz-border-left-colors: ThreeDShadow ThreeDDarkShadow;
> background-color: -moz-Field;
> color: -moz-FieldText;
Is this background-color used?
> listheader {
> -moz-appearance: treeheadercell;
> -moz-box-align: center;
>- border: 2px solid;
>- -moz-border-top-colors: ThreeDHighlight ThreeDLightShadow;
>- -moz-border-right-colors: ThreeDDarkShadow ThreeDShadow;
>- -moz-border-bottom-colors: ThreeDDarkShadow ThreeDShadow;
>- -moz-border-left-colors: ThreeDHighlight ThreeDLightShadow;
> background-color: -moz-Dialog;
> color: -moz-DialogText;
Ditto
>--- a/toolkit/themes/windows/global/popup.css
>+++ b/toolkit/themes/windows/global/popup.css
> menulist > menupopup {
> -moz-appearance: none;
>- border-width: 1px;
>- -moz-border-top-colors: -moz-FieldText;
>- -moz-border-right-colors: -moz-FieldText;
>- -moz-border-bottom-colors: -moz-FieldText;
>- -moz-border-left-colors: -moz-FieldText;
>- padding: 0px;
>- min-width: 0px;
>+ border-color: ThreeDShadow;
>+ padding: 0;
border-color: ThreeDShadow; is redundant now (and padding: 0; too?)
>--- a/toolkit/themes/windows/global/tree.css
>+++ b/toolkit/themes/windows/global/tree.css
> tree {
>- margin: 0px 4px;
>- border: 2px solid;
>- -moz-border-top-colors: ThreeDShadow ThreeDDarkShadow;
>- -moz-border-right-colors: ThreeDHighlight ThreeDLightShadow;
>- -moz-border-bottom-colors: ThreeDHighlight ThreeDLightShadow;
>- -moz-border-left-colors: ThreeDShadow ThreeDDarkShadow;
>+ margin: 0 4px;
>+ border: 1px solid ThreeDShadow;
>+ border-inline-end-color: ThreeDHighlight;
>+ border-bottom-color: ThreeDHighlight;
> background-color: -moz-Field;
> color: -moz-FieldText;
> -moz-appearance: listbox;
> }
Is this border used somewhere?
> treecol[dragging="true"] {
>- -moz-border-top-colors: ThreeDDarkShadow transparent !important;
>- -moz-border-right-colors: ThreeDDarkShadow transparent!important;
>- -moz-border-bottom-colors: ThreeDDarkShadow transparent !important;
>- -moz-border-left-colors: ThreeDDarkShadow transparent !important;
> background-color: ThreeDShadow !important;
> color: ThreeDHighlight !important;
> }
Does setting background-color work here? On Linux it didn't, so I dropped this and just changed the text color to GrayText.
Attachment #8936931 -
Flags: review?(dao+bmo)
Assignee | ||
Comment 29•7 years ago
|
||
(In reply to Dão Gottwald [::dao] from comment #28)
> Comment on attachment 8936931 [details] [diff] [review]
> Bug1423453.patch
>
> >--- a/toolkit/themes/windows/global/listbox.css
> >+++ b/toolkit/themes/windows/global/listbox.css
>
> > listbox {
> > -moz-appearance: listbox;
> > margin: 2px 4px;
> >- border: 2px solid;
> >- -moz-border-top-colors: ThreeDShadow ThreeDDarkShadow;
> >- -moz-border-right-colors: ThreeDHighlight ThreeDLightShadow;
> >- -moz-border-bottom-colors: ThreeDHighlight ThreeDLightShadow;
> >- -moz-border-left-colors: ThreeDShadow ThreeDDarkShadow;
> > background-color: -moz-Field;
> > color: -moz-FieldText;
>
> Is this background-color used?
No, removed.
> > listheader {
> > -moz-appearance: treeheadercell;
> > -moz-box-align: center;
> >- border: 2px solid;
> >- -moz-border-top-colors: ThreeDHighlight ThreeDLightShadow;
> >- -moz-border-right-colors: ThreeDDarkShadow ThreeDShadow;
> >- -moz-border-bottom-colors: ThreeDDarkShadow ThreeDShadow;
> >- -moz-border-left-colors: ThreeDHighlight ThreeDLightShadow;
> > background-color: -moz-Dialog;
> > color: -moz-DialogText;
>
> Ditto
No, removed.
> >--- a/toolkit/themes/windows/global/popup.css
> >+++ b/toolkit/themes/windows/global/popup.css
>
> > menulist > menupopup {
> > -moz-appearance: none;
> >- border-width: 1px;
> >- -moz-border-top-colors: -moz-FieldText;
> >- -moz-border-right-colors: -moz-FieldText;
> >- -moz-border-bottom-colors: -moz-FieldText;
> >- -moz-border-left-colors: -moz-FieldText;
> >- padding: 0px;
> >- min-width: 0px;
> >+ border-color: ThreeDShadow;
> >+ padding: 0;
>
> border-color: ThreeDShadow; is redundant now (and padding: 0; too?)
Yes, removed.
> >--- a/toolkit/themes/windows/global/tree.css
> >+++ b/toolkit/themes/windows/global/tree.css
>
> > tree {
> >- margin: 0px 4px;
> >- border: 2px solid;
> >- -moz-border-top-colors: ThreeDShadow ThreeDDarkShadow;
> >- -moz-border-right-colors: ThreeDHighlight ThreeDLightShadow;
> >- -moz-border-bottom-colors: ThreeDHighlight ThreeDLightShadow;
> >- -moz-border-left-colors: ThreeDShadow ThreeDDarkShadow;
> >+ margin: 0 4px;
> >+ border: 1px solid ThreeDShadow;
> >+ border-inline-end-color: ThreeDHighlight;
> >+ border-bottom-color: ThreeDHighlight;
> > background-color: -moz-Field;
> > color: -moz-FieldText;
> > -moz-appearance: listbox;
> > }
>
> Is this border used somewhere?
The Certificate Manager and Device Manager in their standalone dialogs of TB and SM uses it.
> > treecol[dragging="true"] {
> >- -moz-border-top-colors: ThreeDDarkShadow transparent !important;
> >- -moz-border-right-colors: ThreeDDarkShadow transparent!important;
> >- -moz-border-bottom-colors: ThreeDDarkShadow transparent !important;
> >- -moz-border-left-colors: ThreeDDarkShadow transparent !important;
> > background-color: ThreeDShadow !important;
> > color: ThreeDHighlight !important;
> > }
>
> Does setting background-color work here? On Linux it didn't, so I dropped
> this and just changed the text color to GrayText.
No, removed. And changed the color also to Graytext. This also fixes bug 1327899.
Attachment #8936931 -
Attachment is obsolete: true
Attachment #8937004 -
Flags: review?(dao+bmo)
Comment 30•7 years ago
|
||
> > >--- a/toolkit/themes/windows/global/tree.css
> > >+++ b/toolkit/themes/windows/global/tree.css
> >
> > > tree {
> > >- margin: 0px 4px;
> > >- border: 2px solid;
> > >- -moz-border-top-colors: ThreeDShadow ThreeDDarkShadow;
> > >- -moz-border-right-colors: ThreeDHighlight ThreeDLightShadow;
> > >- -moz-border-bottom-colors: ThreeDHighlight ThreeDLightShadow;
> > >- -moz-border-left-colors: ThreeDShadow ThreeDDarkShadow;
> > >+ margin: 0 4px;
> > >+ border: 1px solid ThreeDShadow;
> > >+ border-inline-end-color: ThreeDHighlight;
> > >+ border-bottom-color: ThreeDHighlight;
> > > background-color: -moz-Field;
> > > color: -moz-FieldText;
> > > -moz-appearance: listbox;
> > > }
> >
> > Is this border used somewhere?
>
> The Certificate Manager and Device Manager in their standalone dialogs of TB
> and SM uses it.
How exactly?
Comment 31•7 years ago
|
||
Comment on attachment 8937004 [details] [diff] [review]
Bug1423453.patch
>--- a/toolkit/themes/osx/global/tree.css
>+++ b/toolkit/themes/osx/global/tree.css
> treecol[dragging="true"] {
>+ color: GrayText !important;
Is !important needed here? It wasn't on Linux.
Assignee | ||
Comment 32•7 years ago
|
||
(In reply to Dão Gottwald [::dao] from comment #31)
> Comment on attachment 8937004 [details] [diff] [review]
> Bug1423453.patch
>
> >--- a/toolkit/themes/osx/global/tree.css
> >+++ b/toolkit/themes/osx/global/tree.css
>
> > treecol[dragging="true"] {
> >+ color: GrayText !important;
>
> Is !important needed here? It wasn't on Linux.
No, removed.
Attachment #8937004 -
Attachment is obsolete: true
Attachment #8937004 -
Flags: review?(dao+bmo)
Attachment #8937009 -
Flags: review?(dao+bmo)
Assignee | ||
Comment 33•7 years ago
|
||
(In reply to Dão Gottwald [::dao] from comment #30)
> > > >--- a/toolkit/themes/windows/global/tree.css
> > > >+++ b/toolkit/themes/windows/global/tree.css
> > >
> > > > tree {
> > > >- margin: 0px 4px;
> > > >- border: 2px solid;
> > > >- -moz-border-top-colors: ThreeDShadow ThreeDDarkShadow;
> > > >- -moz-border-right-colors: ThreeDHighlight ThreeDLightShadow;
> > > >- -moz-border-bottom-colors: ThreeDHighlight ThreeDLightShadow;
> > > >- -moz-border-left-colors: ThreeDShadow ThreeDDarkShadow;
> > > >+ margin: 0 4px;
> > > >+ border: 1px solid ThreeDShadow;
> > > >+ border-inline-end-color: ThreeDHighlight;
> > > >+ border-bottom-color: ThreeDHighlight;
> > > > background-color: -moz-Field;
> > > > color: -moz-FieldText;
> > > > -moz-appearance: listbox;
> > > > }
> > >
> > > Is this border used somewhere?
> >
> > The Certificate Manager and Device Manager in their standalone dialogs of TB
> > and SM uses it.
>
> How exactly?
The border around this tree is from this rule.
Comment 34•7 years ago
|
||
(In reply to Richard Marti (:Paenglab) from comment #33)
> Created attachment 8937010 [details]
> CM.png
>
> (In reply to Dão Gottwald [::dao] from comment #30)
> > > > >--- a/toolkit/themes/windows/global/tree.css
> > > > >+++ b/toolkit/themes/windows/global/tree.css
> > > >
> > > > > tree {
> > > > >- margin: 0px 4px;
> > > > >- border: 2px solid;
> > > > >- -moz-border-top-colors: ThreeDShadow ThreeDDarkShadow;
> > > > >- -moz-border-right-colors: ThreeDHighlight ThreeDLightShadow;
> > > > >- -moz-border-bottom-colors: ThreeDHighlight ThreeDLightShadow;
> > > > >- -moz-border-left-colors: ThreeDShadow ThreeDDarkShadow;
> > > > >+ margin: 0 4px;
> > > > >+ border: 1px solid ThreeDShadow;
> > > > >+ border-inline-end-color: ThreeDHighlight;
> > > > >+ border-bottom-color: ThreeDHighlight;
> > > > > background-color: -moz-Field;
> > > > > color: -moz-FieldText;
> > > > > -moz-appearance: listbox;
> > > > > }
> > > >
> > > > Is this border used somewhere?
> > >
> > > The Certificate Manager and Device Manager in their standalone dialogs of TB
> > > and SM uses it.
> >
> > How exactly?
>
> The border around this tree is from this rule.
Why does that tree not use -moz-appearance: listbox?
Comment 35•7 years ago
|
||
Comment on attachment 8937009 [details] [diff] [review]
Bug1423453.patch
>--- a/toolkit/themes/windows/global/tree.css
>+++ b/toolkit/themes/windows/global/tree.css
> treecol[dragging="true"] {
>+ color: GrayText !important;
Is !important still needed here?
Assignee | ||
Comment 36•7 years ago
|
||
(In reply to Dão Gottwald [::dao] from comment #34)
> (In reply to Richard Marti (:Paenglab) from comment #33)
> > (In reply to Dão Gottwald [::dao] from comment #30)
> > > > The Certificate Manager and Device Manager in their standalone dialogs of TB
> > > > and SM uses it.
> > >
> > > How exactly?
> >
> > The border around this tree is from this rule.
>
> Why does that tree not use -moz-appearance: listbox?
Strange, when I tried it this morning it changed the border color, now not. So removing the border rules.
(In reply to Dão Gottwald [::dao] from comment #35)
> Comment on attachment 8937009 [details] [diff] [review]
> Bug1423453.patch
>
> >--- a/toolkit/themes/windows/global/tree.css
> >+++ b/toolkit/themes/windows/global/tree.css
>
> > treecol[dragging="true"] {
> >+ color: GrayText !important;
>
> Is !important still needed here?
Sorry, forgot it to remove in windows when doing this on osx.
Attachment #8937009 -
Attachment is obsolete: true
Attachment #8937009 -
Flags: review?(dao+bmo)
Attachment #8937019 -
Flags: review?(dao+bmo)
Comment 37•7 years ago
|
||
Attachment #8937019 -
Flags: review?(dao+bmo) → review+
Comment 39•7 years ago
|
||
Pushed by dgottwald@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/4b8c60ce984e
Remove -moz-border-*-colors where possible. r=dao
Keywords: checkin-needed
Comment 40•7 years ago
|
||
bugherder |
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla59
Reporter | ||
Comment 41•7 years ago
|
||
This bug fix has verified in the latest Nightly build (20171217220404) on Win 7, Win 10, macOS 10.13 and Ubuntu 17.10.
Thanks!
Comment 43•7 years ago
|
||
Question. With beta and nightly, the menus have a flat appearance: https://i.imgur.com/zYi7sAu.png
Shouldn't they match the rest of Windows 7 (Classic Theme) and look like it does in 58: https://i.imgur.com/a7mmWNK.png
The regression test I ran pointed to this causing that change. Just wanting to know if going away from 3D appearing menus was on purpose or if it's a bug?
Comment 44•7 years ago
|
||
(In reply to MarkRH from comment #43)
> Question. With beta and nightly, the menus have a flat appearance:
> https://i.imgur.com/zYi7sAu.png
> Shouldn't they match the rest of Windows 7 (Classic Theme) and look like it
> does in 58: https://i.imgur.com/a7mmWNK.png
>
> The regression test I ran pointed to this causing that change. Just wanting
> to know if going away from 3D appearing menus was on purpose or if it's a
> bug?
Kind of on purpose, in that we wanted to simplify the code. There's no Classic theme in recent Windows versions, so it's basically on life support now until Microsoft and Mozilla drop support for Windows 7.
You need to log in
before you can comment on or make changes to this bug.
Description
•