Summary: | [Chromium] Fix default single selection select's popup background on chromium/linux | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | xiyuan <xiyuan> | ||||||||||
Component: | New Bugs | Assignee: | Nobody <webkit-unassigned> | ||||||||||
Status: | RESOLVED FIXED | ||||||||||||
Severity: | Normal | CC: | abarth, dglazkov, eric, tony, webkit.review.bot | ||||||||||
Priority: | P2 | ||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||
Hardware: | PC | ||||||||||||
OS: | Linux | ||||||||||||
Attachments: |
|
Description
xiyuan
2011-03-09 09:42:00 PST
Created attachment 85186 [details]
Proposed patch.
Comment on attachment 85186 [details]
Proposed patch.
A pixel test can't capture this change, right? Is this covered by a manual test? If so, can you mention the test in the ChangeLog. If not, can you add a manual test for this?
(In reply to comment #2) > (From update of attachment 85186 [details]) > A pixel test can't capture this change, right? Is this covered by a manual test? If so, can you mention the test in the ChangeLog. If not, can you add a manual test for this? Select popup (aka dropdown) is not covered in pixel tests because popup is in another render widget and this is not supported by the test infrastructure currently. I am adding a rule so that the default single selection select use #f7f7f7 background color. What do we want to manual test to test? To test that default select and select with size='0'|'1' attribute use the same popup background color? Please clarify a bit. Thanks. (In reply to comment #3) > I am adding a rule so that the default single selection select use #f7f7f7 background color. What do we want to manual test to test? To test that default select and select with size='0'|'1' attribute use the same popup background color? Yes, exactly, that sounds like a good manual test to have. I also wonder if it's possible to use getComputedStyle() to check this in an automated fashion. Maybe not. Created attachment 85365 [details]
Added a layout test.
Added a layout test using getComputedStyle as suggested.
Comment on attachment 85365 [details] Added a layout test. View in context: https://bugs.webkit.org/attachment.cgi?id=85365&action=review > LayoutTests/fast/html/select-dropdown-consistent-background-color.html:20 > + $('result').textContent= Nit: add a space before the = > LayoutTests/fast/html/select-dropdown-consistent-background-color.html:21 > + (default_color == size0_color && default_color == size1_color) ? "PASS" Nit: 4 space indent > LayoutTests/fast/html/select-dropdown-consistent-background-color.html:24 > + layoutTestController.dumpAsText(); Nit: 4 space indent > LayoutTests/fast/html/select-dropdown-consistent-background-color.html:34 > + <option>Item 1</option> > + <option>Item 2</option> Nit: html should probably also use a 4 space indent Comment on attachment 85365 [details] Added a layout test. View in context: https://bugs.webkit.org/attachment.cgi?id=85365&action=review >> LayoutTests/fast/html/select-dropdown-consistent-background-color.html:20 >> + $('result').textContent= > > Nit: add a space before the = Done. >> LayoutTests/fast/html/select-dropdown-consistent-background-color.html:21 >> + (default_color == size0_color && default_color == size1_color) ? "PASS" > > Nit: 4 space indent Done. >> LayoutTests/fast/html/select-dropdown-consistent-background-color.html:24 >> + layoutTestController.dumpAsText(); > > Nit: 4 space indent Done. >> LayoutTests/fast/html/select-dropdown-consistent-background-color.html:34 >> + <option>Item 2</option> > > Nit: html should probably also use a 4 space indent Done. Created attachment 85372 [details]
Fix nits.
Created attachment 85374 [details]
Fix indentation in js functions.
Committed r80777: <http://trac.webkit.org/changeset/80777> http://trac.webkit.org/changeset/80777 might have broken GTK Linux 32-bit Release |