User-Agent: Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en; rv:1.9a1) Gecko/20060831 Camino/1.2+ Build Identifier: Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en; rv:1.9a1) Gecko/20060831 Camino/1.2+ Strange thing. With the 2006083120 trunk build, there is what looks like an image overlayed over the arrows of the select widgets. Screenshot next. Works correctly: 2006083116 build (Maya tinderbox) Fails: 2006083120 (Maya tinderbox) (with default profile and all that...) Reproducible: Always

From Bugzilla. Position of the image varies slightly depending on font-size and width of select widget.

Hmm, the same happens on a Cocoa Firefox build Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.9a1) Gecko/20060831 Minefield/3.0a1 ID:2006083118

Based on that, checkins: http://tinyurl.com/oclf3 I suspect bug 348614.

I suspect that the patch https://bugzilla.mozilla.org/attachment.cgi?id=234422 just needs to be ported to the Cocoa version

*** Bug 351119 has been marked as a duplicate of this bug. ***

Doesn't seem to happen with cocoa-cairo (which has other problems); but I think the patch for 234422 needs to be ported to the HITheme code I wrote as well.

It looks like the problem was that the patch on bug 348614 introduced code into the NS_THEME_DROPDOWN_BUTTON portion of |DrawWidgetBackground()|. That code is what handles the select buttons that have the two arrows on it (up/down arrow). Introducing the code that now sits in NS_THEME_DROPDOWN_BUTTON forces the generic arrow element that only points down to paint over the widget that has the two arrows.

Comment on attachment 236746 [details] [diff] [review] Proposed Patch Doesn't look like the right patch to me. Bug 348614 added support for a standalone <dropmarker> with native theme support. http://xulplanet.com/ndeakin/tests/xts/button/button-dropmarker.xul should display a dropmarker.

(In reply to comment #8) > (From update of attachment 236746 [details] [diff] [review] [edit]) > Doesn't look like the right patch to me. > > Bug 348614 added support for a standalone <dropmarker> with native theme > support. > http://xulplanet.com/ndeakin/tests/xts/button/button-dropmarker.xul > should display a dropmarker. > >I suspect that the patch https://bugzilla.mozilla.org/attachment.cgi?id=234422 >just needs to be ported to the Cocoa version This is the updated version to the patch you pointed out above.

Not clear what you mean. If this is a Cocoa only bug (as I don't see it on the standard Mac build), should it not be the case that nsNativeThemeCocoa.cpp needs to be updated to match the changes in nsNativeThemeMac.cpp?

(In reply to comment #10) > Not clear what you mean. If this is a Cocoa only bug (as I don't see it on the > standard Mac build), should it not be the case that nsNativeThemeCocoa.cpp > needs to be updated to match the changes in nsNativeThemeMac.cpp? > From my research, nsNativeThemeCocoa.cpp is used only for Cairo-Mac.

(In reply to comment #11) > (In reply to comment #10) > > Not clear what you mean. If this is a Cocoa only bug (as I don't see it on the > > standard Mac build), should it not be the case that nsNativeThemeCocoa.cpp > > needs to be updated to match the changes in nsNativeThemeMac.cpp? > > > > From my research, nsNativeThemeCocoa.cpp is used only for Cairo-Mac. > From the Makefile in widget/src/cocoa: ifndef MOZ_ENABLE_CAIRO_GFX MAC_LCPPSRCS += nsNativeThemeMac.cpp \ nsSound.cpp \ $(NULL) endif

We really need to get this bug fixed on trunk. (In reply to comment #10) > If this is a Cocoa only bug (as I don't see it on the > standard Mac build), should it not be the case that nsNativeThemeCocoa.cpp > needs to be updated to match the changes in nsNativeThemeMac.cpp? If there is confusion whether the (in the patch, removed) code is needed still for the Carbonfox, can someone just check their normal firefox build (*non*-cocoa) to verify? If carbon still needs it, you can add a #ifndef MOZ_WIDGET_COCOA.

Here is an updated patch that draws the <dropmarker> element if we aren't using cocoa widgets.

As I said I can't imagine that is the right patch. Did you try the testcase pointed to in comment 8? It should display a native themed dropmarker. The real issue is that Cocoa is using native themed <select> widgets whereas Carbon is not. The real patch needs to modify layout/style/forms.css so that the dropmarker is not shown on Mac.

(In reply to comment #15) > The real issue is that Cocoa is using native themed <select> widgets whereas > Carbon is not. The real patch needs to modify layout/style/forms.css so that > the dropmarker is not shown on Mac. Neil, we're already disabling button drawing on the selects in Cocoa forms: 616 /* make sure nothing paints for the menulist button, since the button is 617 painted as part of the menulist. */ 618 select > input[type="button"], 619 select > input[type="button"]:active { 620 background-image: none !important; 621 } Does the dropmarker have a -moz-foo css constant we can call there to tell it not to display? I'm not sure what/where else in forms.css's Cocoa section we can do something to make the dropmarker not appear on select buttons. (Setting that block to display:none "solves" the problem but breaks the rest of the select sizing....)

Based on the style rules in https://bugzilla.mozilla.org/attachment.cgi?id=234931 themes/classic/global/mac/dropmarker.css This sort of works for me: /* make sure nothing paints for the menulist button, since the button is painted as part of the menulist. */ select > input[type="button"], select > input[type="button"]:active { background-image: none !important; background-color: transparent !important; -moz-appearance: none !important; list-style-image: none !important; width: 0 !important; border: 0 none !important } I don't know how the rules in themes/classic/global/mac/dropmarker.css get applied to the cocoa widgets, but not on a standard build of Minefield

forms.css with the rules in comment 17 added. For testing purposes. It works on my side on all my HTML test cases.

*** Bug 352145 has been marked as a duplicate of this bug. ***

per comment #15 and comment #17 (hopefully it is the right format, first time I do this)

philippe - can you explain what this does, line by line? Thanks.

It seems that only three of those lines are actually necessary, at least in my testing: background-color: transparent !important; /* suppress default gray background */ border: none !important; /* suppress default quasi-bezel */ -moz-appearance: none; /* suppress default aqua-themed appearance */

(In reply to comment #22) > It seems that only three of those lines are actually necessary, at least in my > testing: > > background-color: transparent !important; /* suppress default gray background > */ > border: none !important; /* suppress default quasi-bezel */ > -moz-appearance: none; /* suppress default aqua-themed > appearance */ > You're right. I added the list-style-image more for safety's sake than anything. And thanks for explaining those rules. @Josh Bug 348614 modified (and added) some rules in skin/classic/global/dropmarker.css that are causing the problems for Cocoa widgets (see patch https://bugzilla.mozilla.org/attachment.cgi?id=234931). I neutralised them here. I've tested this on a whole bunch of html testcases both with Camino and Cocoa Firefox without seeing any problems. It also passes the XUL testcase mentioned by Neal in comment #8.

Neil - are you willing to do a first review of this? If you can let me know if this looks good to you I can do a second review in which I actually test it.

*** Bug 354128 has been marked as a duplicate of this bug. ***

*** Bug 354472 has been marked as a duplicate of this bug. ***

*** Bug 354746 has been marked as a duplicate of this bug. ***

Comment on attachment 239285 [details] [diff] [review] proposed patch ? > background-image: none !important; >+ background-color: transparent !important; You should combine these two into a shorthand: background: transparent ! important; >+ -moz-appearance: none !important; >+ list-style-image: none !important; I don't see how list-style-image is at all relevant. Please remove it or convince me that it's needed. >+ border: 0 none !important It's more conventional to write border: none ! important; That said, my real concerns here are bigger: this code shouldn't have been enabled on the trunk because we've turned on cocoa widgets. We shouldn't make the way our form controls respond to author styling totally inconsistent across platforms. (I think Mano was talking about undoing that.) Using nsITheme for form controls should be significantly more consistent (if done right) if you can take that route.

per comment #28 &#65279;(In reply to comment #28) &#65279;> That said, my real concerns here are bigger: this code shouldn't have been > enabled on the trunk because we've turned on cocoa widgets. We shouldn't make > the way our form controls respond to author styling totally inconsistent across > platforms. (I think Mano was talking about undoing that.) Using nsITheme for > form controls should be significantly more consistent (if done right) if you > can take that route. > I believe the goal is to make form controls allow author styling, see bug 320486. [page authors want styling, users are equally divided, some don't care, some prefer no styleable widgets :-)]

landed on trunk. Thanks Philippe!