Bug 72006 – Implement combobox appearance

• Status: RESOLVED
• Resolution: DUPLICATE
• Component: Forms

[Keishi Hattori]

We need to add a new -webkit-appearance combobox to implement datalist for Mac (Bug 27247). It should draw a NSComboBoxCell.

[Keishi Hattori]

Created attachment 114453 [details]
preliminary patch

[Keishi Hattori]

I have a couple of issues.

1. How do you figure out the comboBoxMargins? I tweaked it but it still sometimes gets clipped.
2. There seems to be no way to achieve the push state on the combo box button. Will using Carbon API HIComboBox solve it?
Is there some way to set the highlightBy on the NSComboBoxButtonCell inside the NSComboBoxCell?

[Kent Tamura]

(In reply to comment #2)
> 2. There seems to be no way to achieve the push state on the combo box button. Will using Carbon API HIComboBox solve it?
> Is there some way to set the highlightBy on the NSComboBoxButtonCell inside the NSComboBoxCell?

NSStepperCell for <input type=number> had similar problems, and so we are using HITheme API. See paintStepper() in ThemeMac.mm and Appearance.h.

[Keishi Hattori]

kThemeComboBox seems to work!

[Keishi Hattori]

Created attachment 114613 [details]
Patch

[Keishi Hattori]

I wrote a test but I couldn't find any examples where the test was testing just the -webkit-appearance so I didn't include it in the patch.

I will add a test when applying this style to input element.

[Kent Tamura]

(In reply to comment #6)
> I wrote a test but I couldn't find any examples where the test was testing just the -webkit-appearance so I didn't include it in the patch.
> 
> I will add a test when applying this style to input element.

We should have a test.  fast/forms/range/thumbslider-no-parent-slider.html is an example of -webkit-appearance testing.

[Keishi Hattori]

Created attachment 114616 [details]
added test

[Keishi Hattori]

Comment on attachment 114613 [details]
Patch

I noticed a mistake.

[WebKit Review Bot]

Attachment 114616 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/fast..." exit_code: 1

Source/WebCore/rendering/RenderTheme.cpp:103:  A case label should not be indented, but line up with its switch statement.  [whitespace/indent] [4]
Total errors found: 1 in 8 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Kent Tamura]

Comment on attachment 114616 [details]
added test

View in context: https://bugs.webkit.org/attachment.cgi?id=114616&action=review

> Source/WebCore/platform/mac/ThemeMac.mm:508
> +    HIThemeButtonDrawInfo drawInfo;

The code looks very similar to paintStepper().  Can you share the code with it?

> Source/WebCore/platform/mac/ThemeMac.mm:532
> +    // Center the stepper rectangle in the specified area.

stepper?

> LayoutTests/fast/forms/datalist/combobox-appearance.html:4
> +<script src="../../fast/js/resources/js-test-pre.js"></script>

js-test-pre.js and js-test-post.js are not needed.

> LayoutTests/fast/forms/datalist/combobox-appearance.html:13
> +<p>Test combobox -webkit-appearance</p>

Do not show the unnecessary text.

> LayoutTests/fast/forms/datalist/combobox-appearance.html:20
> +</html>

Please add an expectation image too.

[Simon Pieters (:zcorpan)]

Close as dup of https://bugs.webkit.org/show_bug.cgi?id=187741 ?

[Ahmad Saleem]

Is this needed anymore because it seems that it was duplicate of bug 187741 as per Comment 12.

Appreciate if someone can tag this accordingly. Thanks!

[Ryosuke Niwa]

*** This bug has been marked as a duplicate of bug 187741 ***