Bug 199350 - macOS: <datalist> dropdown shadow is cropped, looks nothing like NSComboBox
Summary: macOS: <datalist> dropdown shadow is cropped, looks nothing like NSComboBox
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Tim Horton
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2019-06-30 20:17 PDT by Tim Horton
Modified: 2019-09-23 15:43 PDT (History)
7 users (show)

See Also:

Attachments
Patch (22.22 KB, patch)
2019-06-30 20:18 PDT, Tim Horton 		no flags Details | Formatted Diff | Diff
Patch (22.23 KB, patch)
2019-06-30 20:20 PDT, Tim Horton 		no flags Details | Formatted Diff | Diff
Patch (22.65 KB, patch)
2019-09-08 19:05 PDT, Tim Horton 		no flags Details | Formatted Diff | Diff
Show Obsolete (2) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Tim Horton 2019-06-30 20:17:12 PDT 
macOS: <datalist> dropdown shadow is cropped, looks nothing like NSComboBox
Comment 1 Tim Horton 2019-06-30 20:18:26 PDT 
Created attachment 373204 [details]
Patch
Comment 2 Tim Horton 2019-06-30 20:20:12 PDT 
Created attachment 373205 [details]
Patch
Comment 3 Maciej Stachowiak 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.)
Comment 4 Tim Horton 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).
Comment 5 Tim Horton 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.
Comment 6 Maciej Stachowiak 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.
Comment 7 Maciej Stachowiak 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,
Comment 8 Tim Horton 2019-09-08 19:05:37 PDT 
Created attachment 378344 [details]
Patch
Comment 9 Tim Horton 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
Comment 10 WebKit Commit Bot 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>
Comment 11 WebKit Commit Bot 2019-09-23 15:42:30 PDT 
All reviewed patches have been landed.  Closing bug.
Comment 12 Radar WebKit Bug Importer 2019-09-23 15:43:18 PDT 
<rdar://problem/55640428>