Bug 25904 - [Chromium] Mac Chromium HTML selects don't initialize their widths
Summary: [Chromium] Mac Chromium HTML selects don't initialize their widths
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: Mac OS X 10.5
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2009-05-20 19:10 PDT by Paul Godavari
Modified: 2009-06-04 16:02 PDT (History)
1 user (show)

See Also:


Attachments
Fix for PopupListBox width initialization (1.34 KB, patch)
2009-05-20 19:22 PDT, Paul Godavari
mjs: review-
Details | Formatted Diff | Diff
Fix for the popup width initialization bug and a manual regression test. (2.49 KB, patch)
2009-05-28 15:34 PDT, Paul Godavari
eric: review-
Details | Formatted Diff | Diff
Updated patch with more description of the problem being solved. (3.19 KB, patch)
2009-06-01 14:40 PDT, Paul Godavari
eric: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Paul Godavari 2009-05-20 19:10:26 PDT
Mac Chromium HTML selects don't set their widths properly at creation time, leading to occasional hit testing errors (making items unable to be selected).
Comment 1 Paul Godavari 2009-05-20 19:22:06 PDT
Created attachment 30519 [details]
Fix for PopupListBox width initialization
Comment 2 Eric Seidel (no email) 2009-05-21 18:05:16 PDT
Comment on attachment 30519 [details]
Fix for PopupListBox width initialization

No way to test this?  We can make hit testing tests from JavaScript quite easily...

document.elementAtPoint()
will hit test.
Comment 3 Maciej Stachowiak 2009-05-22 00:36:50 PDT
Comment on attachment 30519 [details]
Fix for PopupListBox width initialization

Please add a regression test and resubmit.
Comment 4 Paul Godavari 2009-05-28 15:34:55 PDT
Created attachment 30758 [details]
Fix for the popup width initialization bug and a manual regression test.
Comment 5 Eric Seidel (no email) 2009-06-01 13:51:43 PDT
Comment on attachment 30758 [details]
Fix for the popup width initialization bug and a manual regression test.

I don't understand what your test is testing.  Why does it need 26 options?  Ideally the test should have the minimum amount of content to get the point across and no more.

Why is document.elementAtPoint() insufficient for this test?  I take it the <select> element draws at the right place, but the popup itself (a window over the top of Chromium) does not?  In that case, I can see that this would be untestable, but you should explain that in your ChangeLog.
Comment 6 Paul Godavari 2009-06-01 14:40:21 PDT
Created attachment 30841 [details]
Updated patch with more description of the problem being solved.

Updated the patch as per discussion with Eric.
Comment 7 Eric Seidel (no email) 2009-06-04 16:02:05 PDT
Comment on attachment 30841 [details]
Updated patch with more description of the problem being solved.

Looks good.  Thanks.

Committing to http://svn.webkit.org/repository/webkit/trunk ...
	M	WebCore/ChangeLog
	A	WebCore/manual-tests/select-narrow-width.html
	M	WebCore/platform/chromium/PopupMenuChromium.cpp
Committed r44438