Closed Bug 370282 Opened 17 years ago Closed 17 years ago

do not create a dropmarker button inside of comboboxes

Categories

(Core :: Layout: Form Controls, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: jaas, Assigned: jaas)

References

Details

Attachments

(1 file, 1 obsolete file)

We shouldn't be creating a dropmarker button inside of comboboxes, not all platforms work that way (Mac OS X for one).

I thought this might be as easy as #ifdef'ing out the code where the dropmarker button is created in nsComboboxControlFrame::CreateAnonymousContent, but it isn't.
OS: Mac OS X → All
Hardware: Macintosh → All
So what happens on OSX if someone styles the combobox with "-moz-appearance: none"?
I think the native themes and the frame for non-themed comboboxes should draw an image of a dropmarker button in the right place if they want it. There is no other reasonable way of doing this that I can think of. Putting an entirely second element inside of what is usually considered to be one element is never going to work in a cross-platform-friendly way without nasty hacks.
We're putting "entirely second" elements in _all_ our form controls.

The issue with drawing a button yourself in the non-themed case is that you'd have to make it look like a button and behave like a button.  That's not trivial.

I think we should have a way to ask the theme whether it wants a button, and if not to not create one.  But note that I'm not sure how you envision the sizing of the display area to work in that case.  I suppose we could also ask the theme for the size of the thing it's planning to draw in place of the button or something...  But in that case, would it be easier to just style the dropmarker with visibility:hidden in some themes?

If that wouldn't work, can you explain why?  Is there a clear description somewhere of how the Mac native theming for comboboxes is expected to work?
The problem with styling with visibility:hidden is that space is still reserved for the button and it clips the text. Also, it introduces the need for supporting CSS in forms.css again.

I don't know enough about how layout works to envision how the sizing of the display area would work in that case.

Mac native theming for comboboxes works by just drawing an image of the combobox. There is no combobox + a button inside it. There is something that sort of looks like a button on the right side, but it is positioned differently than in Windows and it isn't really a separate entity. I don't think it can even be drawn separately.
> is that space is still reserved for the button 

I had assumed this was in fact desired.  Is it not?

> Mac native theming for comboboxes works by just drawing an image of the
> combobox.

So how is the text drawn?  Or is it not drawn?
We don't want to reserve space unless it is the space over the very right edge of the dropdown button. But even if we had a good way to move that button around on a per-platform basis (read: no supporting CSS hacks), we'd then have to draw it transparently (to expose the button-like thing drawn in its parent frame) and we'd have a size problem because on Mac OS X because dropmarker buttons only come in one size on Mac OS X and that isn't guaranteed to mesh with what we need for comboboxes at all. So, we'd have to have an API for positioning it on a per-platform basis and we'd have to use a new button type like I did in the first patch on bug 369584. Nasty.

The text is drawn by layout somehow.
> We don't want to reserve space 
...
> The text is drawn by layout somehow.

So what's to prevent the text from drawing on top of what you drew for the combobox?  Right now, the text is drawn inside a frame whose sizing is detemined by the width of the <select> and the width of the dropmarker.  How do you propose this sizing be determined on OSX?

For that matter, how do you propose to handle <select>s that are narrower than the dropmarker?
I guess what I want is some indication that there is an actual plan for how to do native-themed comboboxes on OSX; a plan that clearly describes how the combobox should render in various conditions.  Once we have that, we can start figuring out how to go about implementing it.  But until we know what rendering we want (and I still don't know that based on what's been said in this bug), I don't really see how we can discuss implementation.
Attached patch xp code sample fix, v1.0 (obsolete) — Splinter Review
The way rendering should work is that gecko should ask nsITheme for the following information:

- border - we would say that the right border, where our "arrows" render" is about 23 pixels thick so that text always renders to the left of it.

- padding - a few pixels on each side, what you'd expect

This would be the same story for all platforms and generic widgets. Then, when the theme code is asked to render the control they render their button inside the right border. I have actually written the code to test this and it works fine on Mac OS X.

What I'm attaching here are the cross-platform changes we'd need to make for this approach. I'm not 100% sure about all of the CSS I'm removing from forms.css but it works for now. We would have to change the way themes render when asked to draw NS_THEME_DROPDOWN and NS_THEME_DROPDOWN_BUTTON so that they render the dropmarker inside the right side of the control. I have already done this for Mac OS X, but attaching the Mac part of the patch doesn't help us right now.
I think this is a good way to go ... but I wonder if this will affect accessibility, not having the button there.
Isn't the dropdown button itself a "button" in the same sense that the dropmarker inside is? If not, why not?
I don't know. I'm just looking at this comment:
// make someone to listen to the button. If its pressed by someone like  Accessibility
1037   // then open or close the combo box.
It doesn't affect accessibility, but good question. It would have in the past.
That would break all the platforms that do not implement nsITheme or don't implement this part of nsITheme, right?  Do we have a way to check whether a platform does, and fall back on the existing code if it doesn't?

(For example, if the amount of code needed on Windows and Linux to render a dropdown marker is comparable to the amount of code that's used in the combobox frame, we may want to stick with the existing setup, maybe.)

Also, how does this interact with styled comboboxes?  Don't we drop native theming when the page does that?
GTK2 themes don't want a dropdown marker actually. But we won't be using GTK2 themes for comboboxes anytime soon.

The last point is a good one ... we should probably leave the dropdown marker frame in, but not draw it when we use native theming. Then the only new code we have to write, at this point, is Windows support for drawing the marker in nsITheme.
I think Josh's problem is that he needs to change the layout too, not just the painting, when native theming is on...
Right. Having the dropmarker there is a problem because it means that it takes up space in the wrong place in the control, causing the text to get cut off.
OK, then we *have* to leave it in, to be used by the platforms that need it and if native theming is disabled by style changes which don't reconstruct frames. nsComboboxControlFrame::Reflow has to give the button zero size when it's natively themed, and we have to make sure that any style changes that affect that at least do a reflow.
We need that anyway, since nsITheme can already affect, e.g., min widths...

That said, I don't think we handle that, in fact.  Should probably file a separate bug on it.
Attached patch fix v1.0Splinter Review
Assignee: nobody → joshmoz
Attachment #264245 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #265198 - Flags: review?(roc)
Comment on attachment 265198 [details] [diff] [review]
fix v1.0

easy prey!
Attachment #265198 - Flags: superreview+
Attachment #265198 - Flags: review?(roc)
Attachment #265198 - Flags: review+
landed on trunk
Status: ASSIGNED → RESOLVED
Closed: 17 years ago
Resolution: --- → FIXED
How does the theme code know how much space it has where the button would normally be?  Does it query for the default scrollbar width?  You should probably document that assumption somewhere.
(In reply to comment #23)
> How does the theme code know how much space it has where the button would
> normally be?  Does it query for the default scrollbar width?  You should
> probably document that assumption somewhere.

Wait, nevermind... you do need to fix nsComboboxControlFrame::GetPrefWidth, though.
(In reply to comment #24)
> you do need to fix nsComboboxControlFrame::GetPrefWidth,
> though.

Did this slip though the cracks?
Depends on: 381669
I filed bug 381669 on that.
Depends on: 382212
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: