Closed Bug 527682 Opened 13 years ago Closed 13 years ago

Implement -moz-window-shadow values "menu", "tooltip" and "sheet"

Categories

(Core :: Widget: Cocoa, defect)

All
macOS
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla1.9.3a1

People

(Reporter: mstange, Assigned: mstange)

References

Details

(Keywords: polish)

Attachments

(1 file, 1 obsolete file)

Attached patch v1 (obsolete) — Splinter Review
No description provided.
Attachment #411421 - Flags: review?(joshmoz)
Attachment #411421 - Flags: review?(dbaron)
Attachment #411421 - Flags: review?(dao)
Shouldn't -moz-appearance:menupopup imply -moz-window-shadow:menu and -moz-appearance:tooltip imply -moz-window-shadow:tooltip?
That would make sense but is harder to implement in a clean way. Another (albeit minor) advantage of having those properties separate is that it gives themers a little more freedom.
It would be consistent with Windows and Linux and thereby allow cross-platform styling, which I think is partly the point of -moz-appearance. With that in mind, I don't regard splitting appearance and shadow as an advantageous freedom.
(In reply to comment #1)
> Shouldn't -moz-appearance:menupopup imply -moz-window-shadow:menu

Maybe I should clarify that -moz-appearance:menupopup should not actually result in -moz-window-shadow:menu in the computed style. In my mind "menu" shouldn't exist at all, but "default" should be interpreted according to -moz-appearance.
(In reply to comment #3)
> It would be consistent with Windows and Linux and thereby allow cross-platform
> styling,

Do Windows and Linux change the shadow style dependent on the value of -moz-appearance? I don't think so.
I'm not opposed to the idea of having -moz-appearance influence the shadow style; it's just that the implementation is not obvious to me at the moment, so I'd like to do it the easy way first.

Moreover, I'm not disallowing cross-platform styling here. If a themer cares to set -moz-window-shadow, he gets a small upgrade in shadow correctness; if he doesn't do it, he'll still get the default shadow and it won't look any worse than it has since forever.

> which I think is partly the point of -moz-appearance.

It might have been the point of it when it was invented long ago, but the current state of affairs is the complete opposite. Correct cross-platform styling using -moz-appearance is nearly impossible in a lot of other cases. But this bug is not the right place to discuss that problem.
Blocks: 528585
Attachment #411421 - Flags: review?(dbaron) → review+
Comment on attachment 411421 [details] [diff] [review]
v1

r=dbaron on the layout/style parts (or did you want me to look at more?)
Attached patch v2Splinter Review
This doesn't set shadows on menus and tooltips.
Attachment #411421 - Attachment is obsolete: true
Attachment #417064 - Flags: review?(dao)
Attachment #411421 - Flags: review?(joshmoz)
Attachment #411421 - Flags: review?(dao)
Attachment #417064 - Flags: review?(joshmoz)
Blocks: 534161
(In reply to comment #1)
> Shouldn't -moz-appearance:menupopup imply -moz-window-shadow:menu and
> -moz-appearance:tooltip imply -moz-window-shadow:tooltip?

I filed bug 534161.
Comment on attachment 417064 [details] [diff] [review]
v2

>+  eCSSKeyword_menu, NS_STYLE_WINDOW_SHADOW_MENU,
>+  eCSSKeyword_tooltip, NS_STYLE_WINDOW_SHADOW_TOOLTIP,

Given bug 534161, what's the point of exposing these?
Attachment #417064 - Flags: review?(dao) → review+
A theme might want to have blue tooltips and still have the right tooltip shadow. Or it might want to have green, slightly transparent context menus with a blurred background. (In bug 498258 I'm going to bind background blurring to the shadow style.)
Moreover, exposing these values is only one line per value, so there's not much reason not to do it.
Are these shadows going to follow the border radius? If not, this doesn't seem quite right, and a custom shadow should be used together with the custom background.

There comes a cost with every css value, as it needs to be maintained in the future, authors might expect it to work cross-platform etc.
(In reply to comment #11)
> Are these shadows going to follow the border radius?

They are, and also every other form that's rendered inside the window.

> and a custom shadow should be used together with the custom background.

I'm not sure you can achieve the same effect with a custom shadow. With -moz-window-shadow, clicking outside the menu into the shadow closes the menu. But if you use e.g. -moz-box-shadow, you need to have padding inside the menu so that the shadow isn't clipped; and that will make the shadow part of the menu click region, so clicking there won't close it. You'll also have problems with position offsets etc.

> There comes a cost with every css value,

Sure, but it seems very low in this case.
Attachment #417064 - Flags: review?(joshmoz) → review+
http://hg.mozilla.org/mozilla-central/rev/957fabf996ae
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9.3a1
You need to log in before you can comment on or make changes to this bug.