Bug 199350

Summary: macOS: <datalist> dropdown shadow is cropped, looks nothing like NSComboBox
Product: WebKit Reporter: Tim Horton <thorton>
Component: New BugsAssignee: Tim Horton <thorton>
Status: RESOLVED FIXED    
Severity: Normal CC: bdakin, commit-queue, javan, mjs, timothy, webkit-bug-importer, wenson_hsieh
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch
none
Patch none

Tim Horton
Reported 2019-06-30 20:17:12 PDT
macOS: <datalist> dropdown shadow is cropped, looks nothing like NSComboBox
Attachments
Patch (22.22 KB, patch)
2019-06-30 20:18 PDT, Tim Horton 		no flags
DetailsFormatted DiffDiff
Patch (22.23 KB, patch)
2019-06-30 20:20 PDT, Tim Horton 		no flags
DetailsFormatted DiffDiff
Patch (22.65 KB, patch)
2019-09-08 19:05 PDT, Tim Horton 		no flags
DetailsFormatted DiffDiff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
Tim Horton
Comment 1 2019-06-30 20:18:26 PDT
Created attachment 373204 [details] Patch
Tim Horton
Comment 2 2019-06-30 20:20:12 PDT
Created attachment 373205 [details] Patch
Maciej Stachowiak
Comment 3 2019-07-18 21:58:11 PDT
Comment on attachment 373205 [details] Patch How testable is datalist? Is there a way to make a test case for these improvements? (Everything mentioned in the ChangeLog sounds great though.)
Tim Horton
Comment 4 2019-07-18 23:44:08 PDT
(In reply to Maciej Stachowiak from comment #3) > Comment on attachment 373205 [details] > Patch > > How testable is datalist? Is there a way to make a test case for these > improvements? (Everything mentioned in the ChangeLog sounds great though.) I believe (will have to check) that there is some testing support for basic datalist functionality, but I don’t have a great plan for the testing of the UI layout of the dropdown (which is native UI, in a separate window, in the UI process).
Tim Horton
Comment 5 2019-07-18 23:45:25 PDT
maybe we should give WKTR a mode where it dumps the geometry of subwindows (that would help with TextIndicator testing too). The UI inside the pop up, though... I dunno.
Maciej Stachowiak
Comment 6 2019-08-10 14:30:13 PDT
Comment on attachment 373205 [details] Patch Given the difficulty of testing and the values of the improvement it's fine to land without a test. I read every line and did not see any problems, but I'm not very familiar with the details of AppKit stuff used here.
Maciej Stachowiak
Comment 7 2019-08-10 14:31:10 PDT
Comment on attachment 373205 [details] Patch I'm just going to r+ this and leave it to the patch author's judgment whether further review is merited,
Tim Horton
Comment 8 2019-09-08 19:05:37 PDT
Created attachment 378344 [details] Patch
Tim Horton
Comment 9 2019-09-08 19:05:55 PDT
rebased on master. also mjs did not actually hit the r+ button, if someone else wants to :P
WebKit Commit Bot
Comment 10 2019-09-23 15:42:28 PDT
Comment on attachment 378344 [details] Patch Clearing flags on attachment: 378344 Committed r250260: <https://trac.webkit.org/changeset/250260>
WebKit Commit Bot
Comment 11 2019-09-23 15:42:30 PDT
All reviewed patches have been landed. Closing bug.
Radar WebKit Bug Importer
Comment 12 2019-09-23 15:43:18 PDT
<rdar://problem/55640428>
Note You need to log in before you can comment on or make changes to this bug.