Bug 72006

Summary: Implement combobox appearance
Product: WebKit Reporter: Keishi Hattori <keishi>
Component: FormsAssignee: Keishi Hattori <keishi>
Status: RESOLVED DUPLICATE    
Severity: Normal CC: ahmad.saleem792, akeerthi, ap, bfulgham, hyatt, macpherson, mike, rniwa, simon.fraser, syoichi, tkent, webkit.review.bot, zalan, zcorpan
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Mac   
OS: All   
Bug Depends on:    
Bug Blocks: 27247    
Attachments:
Description Flags
preliminary patch
none
Patch
none
added test tkent: review-

Keishi Hattori
Reported 2011-11-10 01:25:51 PST
We need to add a new -webkit-appearance combobox to implement datalist for Mac (Bug 27247). It should draw a NSComboBoxCell.
Attachments
preliminary patch (7.56 KB, patch)
2011-11-10 01:45 PST, Keishi Hattori 		no flags
DetailsFormatted DiffDiff
Patch (7.14 KB, patch)
2011-11-10 18:16 PST, Keishi Hattori 		no flags
DetailsFormatted DiffDiff
added test (8.60 KB, patch)
2011-11-10 19:44 PST, Keishi Hattori 		tkent: review-
DetailsFormatted DiffDiff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
Keishi Hattori
Comment 1 2011-11-10 01:45:17 PST
Created attachment 114453 [details] preliminary patch
Keishi Hattori
Comment 2 2011-11-10 01:51:11 PST
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
Comment 3 2011-11-10 02:10:47 PST
(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
Comment 4 2011-11-10 02:32:10 PST
kThemeComboBox seems to work!
Keishi Hattori
Comment 5 2011-11-10 18:16:31 PST
Created attachment 114613 [details] Patch
Keishi Hattori
Comment 6 2011-11-10 18:18:42 PST
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
Comment 7 2011-11-10 18:22:23 PST
(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
Comment 8 2011-11-10 19:44:27 PST
Created attachment 114616 [details] added test
Keishi Hattori
Comment 9 2011-11-10 19:51:46 PST
Comment on attachment 114613 [details] Patch I noticed a mistake.
WebKit Review Bot
Comment 10 2011-11-10 19:52:17 PST
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 11 2011-11-10 19:53:47 PST
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)
Comment 12 2019-01-23 12:22:47 PST
Close as dup of https://bugs.webkit.org/show_bug.cgi?id=187741 ?
Ahmad Saleem
Comment 13 2022-08-17 14:02:38 PDT
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
Comment 14 2022-08-17 22:14:05 PDT
*** This bug has been marked as a duplicate of bug 187741 ***
Note You need to log in before you can comment on or make changes to this bug.