| Summary: | Setting background-color of an input[type="search"] does not work | ||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Emily <ademily> | ||||||||||||||||||
| Component: | Forms | Assignee: | Nobody <webkit-unassigned> | ||||||||||||||||||
| Status: | REOPENED --- | ||||||||||||||||||||
| Severity: | Normal | CC: | ap, buildbot, cdumez, commit-queue, darin, esprehn+autocc, glenn, gyuyoung.kim, jonlee, kling, kondapallykalyan, mats.ahlgren, mitz, priyajeet.hora, rakuco, rniwa, santosh.ma, s+webkit | ||||||||||||||||||
| Priority: | P3 | ||||||||||||||||||||
| Version: | 525.x (Safari 3.2) | ||||||||||||||||||||
| Hardware: | All | ||||||||||||||||||||
| OS: | All | ||||||||||||||||||||
| URL: | http://jsbin.com/omiYUmi/1/edit | ||||||||||||||||||||
| Attachments: |
|
||||||||||||||||||||
|
Description
Emily
2013-08-17 22:13:21 PDT
Created attachment 214006 [details]
Patch
Comment on attachment 214006 [details] Patch Attachment 214006 [details] did not pass mac-ews (mac): Output: http://webkit-queues.appspot.com/results/3947213 New failing tests: fast/forms/search-styled.html Created attachment 214013 [details]
Archive of layout-test-results from webkit-ews-02 for mac-mountainlion
The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: webkit-ews-02 Port: mac-mountainlion Platform: Mac OS X 10.8.5
Comment on attachment 214006 [details] Patch Attachment 214006 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.appspot.com/results/3945194 New failing tests: fast/forms/search-styled.html Created attachment 214017 [details]
Archive of layout-test-results from webkit-ews-11 for mac-mountainlion-wk2
The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: webkit-ews-11 Port: mac-mountainlion-wk2 Platform: Mac OS X 10.8.5
Comment on attachment 214006 [details] Patch Attachment 214006 [details] did not pass mac-ews (mac): Output: http://webkit-queues.appspot.com/results/3954217 New failing tests: fast/forms/search-styled.html Created attachment 214020 [details]
Archive of layout-test-results from webkit-ews-07 for mac-mountainlion
The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: webkit-ews-07 Port: mac-mountainlion Platform: Mac OS X 10.8.5
Created attachment 214149 [details]
Patch
Comment on attachment 214149 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=214149&action=review > Source/WebCore/ChangeLog:9 > + it doesnot change the background-color of field. Its happening becasue Typo: doesnot. > LayoutTests/fast/forms/search/search-field-background-color.html:10 > +<input type="search" id="test" > Why is there a space before >? > LayoutTests/fast/forms/search/search-field-background-color.html:17 > +if (window.testRunner) { > + testRunner.dumpAsText(); > +} No brackets around a single line statement. > LayoutTests/fast/forms/search/search-field-background-color.html:20 > +if (document.defaultView.getComputedStyle(test, null).getPropertyValue('background-color') == "rgb(255, 0, 0)") Get rid of "document.defaultView." Was this done intentionally? Would background colors make some search inputs unreadable? According to http://trac.webkit.org/browser/trunk/WebCore/rendering/RenderTheme.cpp?rev=18248, this wasn't intentional. Also, we should probably have a pixel test for this. Created attachment 214173 [details]
Patch
Comment on attachment 214173 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=214173&action=review > LayoutTests/fast/forms/search/search-field-background-color.html:20 > +if (getComputedStyle(test, null).getPropertyValue('background-color') == "rgb(255, 0, 0)") > + result.innerHTML = "PASS"; Why are we not using shouldBe in js-test-pre.js here? Also, please add a pixel test to make sure it's rendered properly. Created attachment 214224 [details]
Patch
Comment on attachment 214224 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=214224&action=review > LayoutTests/fast/forms/search/search-field-background-color.html:16 > + testRunner.dumpAsText(true); Please add -expected.png for at least one platform. I'm confused. How can there be an expected.png for a test only text? (In reply to comment #17) > I'm confused. How can there be an expected.png for a test only text? dumpAsText(true) generates -expected.png whereas dumpAsText(false) doesn't. Created attachment 214232 [details]
Patch
Comment on attachment 214232 [details] Patch Clearing flags on attachment: 214232 Committed r157443: <http://trac.webkit.org/changeset/157443> All reviewed patches have been landed. Closing bug. *** Bug 33636 has been marked as a duplicate of this bug. *** *** Bug 45417 has been marked as a duplicate of this bug. *** *** Bug 53479 has been marked as a duplicate of this bug. *** (In reply to comment #20) > (From update of attachment 214232 [details]) > Clearing flags on attachment: 214232 > > Committed r157443: <http://trac.webkit.org/changeset/157443> This cased bug 126295. Seems like the comment just above the uncommented line was ignored! I reverted the incorrect change in r164145. |