Editable menulists in Vista are rendered in the style of non-editable menulists

RESOLVED FIXED in mozilla7

Status

()

Core
Widget: Win32
--
minor
RESOLVED FIXED
9 years ago
6 years ago

People

(Reporter: Kai Liu, Assigned: Kai Liu)

Tracking

Trunk
mozilla7
x86
Windows Vista
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(4 attachments, 2 obsolete attachments)

(Assignee)

Description

9 years ago
Created attachment 318039 [details]
testcase

In chrome, non-editable menulists should be rendered in a new button-like style in Vista, while editable menulists should be rendered in the normal textbox style.  However, Gecko renders both editable and non-editable menulists in the new button-like style in Vista.

This does not apply to HTML content, as "menulists" in HTML (<select>) are always rendered in the textbox style.
(Assignee)

Comment 1

9 years ago
Created attachment 318040 [details]
native vs. chrome screenshot
Assignee: nobody → netzen
Created attachment 536266 [details] [diff] [review]
Editable menulists were not using -moz-appearance: menulist-textfield;

Attached a patch which fixes the problem. 

The problem was that menulist's -moz-appearance was being inherited.

I also checked to see if this problem was on OS X, Ubuntu, and Kubuntu, but everything worked normal on those with default settings and no code changes.
I also verified in the other menulist.css files for other platforms and they all have similar settings that I came up with, winstripe was the only one missing it.
Attachment #536266 - Flags: review?(dao)

Updated

6 years ago
Component: Widget: Win32 → Themes
Product: Core → Toolkit
QA Contact: win32 → themes
Comment on attachment 536266 [details] [diff] [review]
Editable menulists were not using -moz-appearance: menulist-textfield;

This doesn't really look as expected... The win32 widget support for menulist-textfield seems to be broken / incomplete. We either need that fixed or should use -moz-appearance: textfield here.
Created attachment 539176 [details]
screenshot with patch

Updated

6 years ago
Attachment #536266 - Flags: review?(dao) → review-
(Assignee)

Comment 5

6 years ago
(In reply to comment #3)
> We either need that fixed or should use -moz-appearance: textfield here.

Or option 3: We could keep using -moz-appearance: menulist. :)

The menulist variations do not hinge solely on editable/non-editable:

1) Rendered like a textfield with a discrete button: Classic or pre-Aero or editable or non-editable in HTML content.  And with a different dropmarker style if using Aero and the outer frame is custom-styled.

2) Rendered like a button: non-editable Aero in XUL

If we want to expose these variations in CSS, then we should do it right and not part-way.  For example, we would need to do something about the dropmarker.  Either create another menulist-button appearance (e.g., menulist-textfield-button) or do what Linux does and hide the dropmarker element in the case of the button-ish style and draw the dropmarker directly in the menulist DrawWidgetBackground.

The status quo is to not expose the menulist variations in CSS on Windows: we use only the menulist and menulist-button appearances and let the native theme code sort out how it should all be rendered.  Exposing these variations in CSS seems cumbersome to me (and what would the benefit be?), so I am in favor of maintaining this status quo.  Yes, this would leave the menulist-textfield unimplemented for Windows, but does the custom theme community care about this one?

The current native theme code for the menulist frame and dropmarker does a check to see if it's in HTML content and uses the textfield-like style if it is.  So the quick fix would be to do the same for non-editable menulists.
Component: Themes → Widget: Win32
Product: Toolkit → Core
QA Contact: themes → win32
(Assignee)

Comment 6

6 years ago
Created attachment 539255 [details] [diff] [review]
render editable menulists with CBP_DROPBORDER instead of CBP_DROPFRAME

Sorry to steal this bug from you, Brian!

I had recently been poking around menulist-related code for another bug, so this patch was already kicking around in my head.

This implements the quick fix that I advocated for in comment 5.  This leaves the menulist-textfield appearance unimplemented, which I think is the sane thing to do.
Assignee: netzen → kliu
Attachment #536266 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #539255 - Flags: review?(jmathies)
No problem at all, I wasn't going to do another fix until tonight.

Comment 8

6 years ago
Comment on attachment 539255 [details] [diff] [review]
render editable menulists with CBP_DROPBORDER instead of CBP_DROPFRAME

I'm fine with the code change here but I'd like Dao to approve the look and feel changes.
Attachment #539255 - Flags: superreview?(dao)
Attachment #539255 - Flags: review?(jmathies)
Attachment #539255 - Flags: review+
Comment on attachment 539255 [details] [diff] [review]
render editable menulists with CBP_DROPBORDER instead of CBP_DROPFRAME

(I'm not a super-reviewer.)
Attachment #539255 - Flags: superreview?(dao) → review+
(Assignee)

Comment 10

6 years ago
Created attachment 541928 [details] [diff] [review]
[for checkin] render editable menulists with CBP_DROPBORDER instead of CBP_DROPFRAME [r=jmathies+dao]

Resynchronized with m-c's tip and updated commit message.
(Assignee)

Updated

6 years ago
Attachment #539255 - Attachment is obsolete: true
(Assignee)

Updated

6 years ago
Keywords: checkin-needed
http://hg.mozilla.org/mozilla-central/rev/9df0849fa705
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → mozilla7
You need to log in before you can comment on or make changes to this bug.