395565 - spinner of cocoa has wrong size

Status: RESOLVED FIXED

(Core :: Widget: Cocoa, defect)

Description

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: Patch rv1.0

spinner of cocoa is not getting native control size. Therefore, up/down button rect is not match to the native theme.

this blocks bug 380026, maybe.

Comment 1

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Håkan Waara:

Please use UTF-8 (without BOM) when you write a patch. 'å' is not an ASCII character. Therefore, non-Latin-1 characters cannot be used with your signature.

Comment 2

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

er, this is not a rendering bug. the rendering result is correct, but the box sizes are wrong, therefore, the event handling is failed.

Comment 3

[[e-mail]]

Comment on attachment 280232 [details] [diff] [review]
Patch rv1.0

Wrong Neil, perhaps?

Comment 4

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

you have reviewed it in bug 155053, therefore, I requested to you ;)

Comment 5

[Josh Aas]

Comment on attachment 280232 [details] [diff] [review]
Patch rv1.0

I assume you tested this and it works, but why did you choose "kThemeMetricLittleArrowsWidth"? Is that really the right metric?

Comment 6

[Håkan Waara]

(In reply to comment #5)
> (From update of attachment 280232 [details] [diff] [review])
> I assume you tested this and it works, but why did you choose
> "kThemeMetricLittleArrowsWidth"? Is that really the right metric?

"Little arrows" (in Carbon world) == spinbutton in XUL world, I think?

Comment 7

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

I think this patch works fine for me.

http://developer.apple.com/documentation/Carbon/Reference/Appearance_Manager/Reference/reference.html#//apple_ref/doc/c_ref/kThemeMetricLittleArrowsHeight
> kThemeMetricLittleArrowsHeight
> 
>     The height of a little arrows control.
> 
>     Available in Mac OS X v10.0 and later.
> kThemeMetricLittleArrowsWidth
> 
>     The width of a little arrows control.
> 
>     Available in Mac OS X v10.0 and later.

The little arrows control means:

http://developer.apple.com/documentation/Carbon/Reference/Appearance_Manager/Reference/reference.html#//apple_ref/doc/c_ref/kThemeIncDecButton

> kThemeIncDecButton
> 
>     Identifies an increment/decrement or "little arrows" button. This button has the appearance of two separate buttons—one containing an upward-pointing triangle and the other containing a downward-pointing triangle—placed back to back. This button should not be confused with the arrow button.
> 
>     Available in Mac OS X v10.0 and later.

Comment 8

[Robert O'Callahan (:roc) (email my personal email if necessary)]

 spinbuttons {
-  height: 24px;

Why are you removing this? Won't this break non-native-theme situations?

Comment 9

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

(In reply to comment #8)
>  spinbuttons {
> -  height: 24px;
> 
> Why are you removing this? Won't this break non-native-theme situations?

In my environment, the height returns 21px. 24px is too large. And shouldn't specify themselves when they are creating non-native-theme?

Comment 10

[[e-mail]]

(In reply to comment #8)
>  spinbuttons {
> -  height: 24px;
> 
> Why are you removing this? Won't this break non-native-theme situations?
> 

Pinstripe never supported a non-native-theme situation; Gnomestripe doesn't either. Winstripe is better in this respect, but it has bugs.

Comment 11

[Robert O'Callahan (:roc) (email my personal email if necessary)]

But since *aIsOverridable is set to PR_FALSE, the CSS height will be ignored, right? So it won't hurt to leave that line in, and it might help.

Comment 12

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: Patch rv1.1

er, o.k. you're right.

Comment 13

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

checked-in.