Closed Bug 373696 Opened 15 years ago Closed 14 years ago

style toolbar separators natively

Categories

(Toolkit :: Themes, defect)

x86
Windows XP
defect
Not set
normal

Tracking

()

VERIFIED FIXED
mozilla1.9alpha8

People

(Reporter: dao, Assigned: zeniko)

References

Details

Attachments

(4 files, 3 obsolete files)

Attached patch patch (obsolete) — Splinter Review
I'll submit some screenshots later.
I was actually about to remove the horizontal borders entirely in my personal setup, as they hardly fulfil a practical function. I found them visually distracting. Then I noticed that the Explorer is using a ThreeDLightShadow-like color, and I think that should be the default appearance for Moz apps under WinXP / Luna. It's probably also the right thing for Vista; I'm not sure about Windows Classic.
Attachment #258369 - Flags: ui-review?(beltzner)
Attached image trunk layout/rendering bug (obsolete) —
There's a corrupted line in the toolbar right above the tab bar. That's not really related to this change, though it becomes more obvious.
I was unable to find its origin with DOMi. Maybe it's a core bug -- I'm thinking of bug 371434.
Assignee: dao → nobody
Component: OS Integration → Toolbars and Toolbar Customization
Flags: ui-review?(beltzner)
Product: Firefox → Toolkit
QA Contact: os.integration → toolbars
Attachment #258369 - Flags: first-review?(beltzner)
Attachment #258369 - Flags: first-review?(beltzner)
Attachment #258345 - Flags: second-review?(gavin.sharp)
Attachment #258345 - Flags: first-review?(mconnor)
Attachment #258369 - Attachment description: before/after/reference screenshots → Firefox before/after/reference screenshots
(In reply to comment #2)
> Created an attachment (id=258373) [details]
> trunk layout/rendering bug

This applies to Thunderbird as well.
Assignee: nobody → dao
Status: NEW → ASSIGNED
Attachment #258345 - Flags: first-review?(mconnor) → review?(mconnor)
Attachment #258345 - Flags: second-review?(gavin.sharp) → review?
Attachment #258345 - Flags: review? → review?(gavin.sharp)
Requesting blocking in order to get this on the radar. I'm fine with a WONTFIX, but somebody should actually make that decision.
Flags: blocking1.9?
Target Milestone: --- → mozilla1.9alpha6
Summary: toolbar shadow is too dark / obtrusive → make toolbar shadows fainter
Flags: blocking1.9?
I think this was fixed by bug 337771.
Assignee: dao → nobody
Status: ASSIGNED → NEW
Depends on: 337771
Target Milestone: mozilla1.9alpha6 → mozilla1.9beta1
Status: NEW → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
Attachment #258345 - Flags: review?(mconnor)
Attachment #258345 - Flags: review?(gavin.sharp)
Ah, not entirely fixed. Although toolbar shadows use the light shadow, toolbarseparators still use the dark one.
Severity: enhancement → normal
Status: RESOLVED → REOPENED
Component: Toolbars and Toolbar Customization → Themes
Product: Toolkit → Core
Resolution: FIXED → ---
Blocks: 337771
No longer depends on: 337771
Attachment #258373 - Attachment is obsolete: true
Flags: blocking1.9?
Pretty simple patch, except that there's another magic number creeping in (the default code path in GetMinimumWidgetSize somehow returns either too large or too small a value for what we need to look right) and that the classic CSS version has to use overridable borders instead of margins (those come included with the natively themed separator).
Assignee: nobody → zeniko
Status: REOPENED → ASSIGNED
Attachment #273238 - Flags: superreview?(neil)
Attachment #273238 - Flags: review?(emaijala)
Attachment #258345 - Attachment is obsolete: true
updating summary to what this bug is actually about now.
Summary: make toolbar shadows fainter → style toolbar separators natively
Comment on attachment 273238 [details] [diff] [review]
style toolbar separators natively

> toolbarseparator {
>-  margin : 2px 0.2em;
>-  border-right : 1px solid ThreeDHighlight;
>-  border-left : 1px solid ThreeDShadow;
>-  width : 2px;
>+  -moz-appearance: separator;
>+  border-top: 2px solid transparent;
>+  border-bottom: 2px solid transparent;
>+  border-left: 3px solid transparent;
>+  border-right: 3px solid transparent;
>+  -moz-border-left-colors  : transparent transparent ThreeDShadow;
>+  -moz-border-right-colors : transparent transparent ThreeDHighlight;
> }
I'm assuming here that the way the theming works is that you're not doing anything on Windows 2000 or XP Classic and so this CSS takes effect instead.
Attachment #273238 - Flags: superreview?(neil) → superreview+
Comment on attachment 273238 [details] [diff] [review]
style toolbar separators natively

r=emaijala
Attachment #273238 - Flags: review?(emaijala) → review+
Keywords: checkin-needed
Attachment #273238 - Flags: approval1.9?
Attachment #273238 - Flags: approval1.9? → approval1.9+
Attached patch updated to trunkSplinter Review
Attachment #273238 - Attachment is obsolete: true
mozilla/widget/src/windows/nsNativeThemeWin.cpp      3.92
mozilla/toolkit/themes/winstripe/global/toolbar.css  1.16
Status: ASSIGNED → RESOLVED
Closed: 15 years ago14 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
FWIW, the resizer (between location bar and search) look bad since this landed.
I assume this patch may have predated addition of the resizer?
(In reply to comment #13)
> FWIW, the resizer (between location bar and search) look bad since this landed.
It really shouldn't look any _worse_ than before. Anyway, should bug 391128 not take care of your issue, please just file another polish bug.
After this patch landed I have no toolbar separators at all with non MS themes. And with MS themes the separators are not padded at the bottom.
Attached image No separators
Depends on: 394142
Ali: Please file a follow up bug and make sure to include screenshots from Windows Explorer(!) for comparison (separators are indeed expected to look differently than before this bug, so there's no need to add screenshots from older Minefield builds).
> Ali: Please file a follow up bug
Never mind - that's bug 394142.
Flags: blocking1.9?
Product: Core → SeaMonkey
Product: SeaMonkey → Toolkit
QA Contact: toolbars → themes
Verified fixed with Mozilla/5.0 (Windows; U; Windows NT 5.1; de; rv:1.9.0.10) Gecko/2009042316 Firefox/3.0.10 (.NET CLR 3.5.30729) ID:2009042316
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.