Last Comment Bug 431078 - Editable menulists in Vista are rendered in the style of non-editable menulists
: Editable menulists in Vista are rendered in the style of non-editable menulists
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: Widget: Win32 (show other bugs)
: Trunk
: x86 Windows Vista
: -- minor (vote)
: mozilla7
Assigned To: Kai Liu
:
Mentors:
Depends on:
Blocks: 429310
  Show dependency treegraph
 
Reported: 2008-04-27 12:29 PDT by Kai Liu
Modified: 2011-07-01 05:43 PDT (History)
4 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
testcase (869 bytes, application/vnd.mozilla.xul+xml)
2008-04-27 12:29 PDT, Kai Liu
no flags Details
native vs. chrome screenshot (30.73 KB, image/png)
2008-04-27 12:40 PDT, Kai Liu
no flags Details
Editable menulists were not using -moz-appearance: menulist-textfield; (999 bytes, patch)
2011-05-31 03:48 PDT, Brian R. Bondy [:bbondy]
dao+bmo: review-
Details | Diff | Splinter Review
screenshot with patch (2.05 KB, image/png)
2011-06-14 04:49 PDT, Dão Gottwald [:dao]
no flags Details
render editable menulists with CBP_DROPBORDER instead of CBP_DROPFRAME (5.11 KB, patch)
2011-06-14 11:14 PDT, Kai Liu
jmathies: review+
dao+bmo: review+
Details | Diff | Splinter Review
[for checkin] render editable menulists with CBP_DROPBORDER instead of CBP_DROPFRAME [r=jmathies+dao] (5.29 KB, patch)
2011-06-25 07:04 PDT, Kai Liu
no flags Details | Diff | Splinter Review

Description Kai Liu 2008-04-27 12:29:17 PDT
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.
Comment 1 Kai Liu 2008-04-27 12:40:03 PDT
Created attachment 318040 [details]
native vs. chrome screenshot
Comment 2 Brian R. Bondy [:bbondy] 2011-05-31 03:48:44 PDT
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.
Comment 3 Dão Gottwald [:dao] 2011-06-14 04:49:08 PDT
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.
Comment 4 Dão Gottwald [:dao] 2011-06-14 04:49:54 PDT
Created attachment 539176 [details]
screenshot with patch
Comment 5 Kai Liu 2011-06-14 10:14:56 PDT
(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.
Comment 6 Kai Liu 2011-06-14 11:14:41 PDT
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.
Comment 7 Brian R. Bondy [:bbondy] 2011-06-14 11:31:10 PDT
No problem at all, I wasn't going to do another fix until tonight.
Comment 8 Jim Mathies [:jimm] 2011-06-23 09:38:47 PDT
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.
Comment 9 Dão Gottwald [:dao] 2011-06-24 06:20:30 PDT
Comment on attachment 539255 [details] [diff] [review]
render editable menulists with CBP_DROPBORDER instead of CBP_DROPFRAME

(I'm not a super-reviewer.)
Comment 10 Kai Liu 2011-06-25 07:04:24 PDT
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.
Comment 11 Dão Gottwald [:dao] 2011-07-01 05:43:52 PDT
http://hg.mozilla.org/mozilla-central/rev/9df0849fa705

Note You need to log in before you can comment on or make changes to this bug.