Bug 119967 - Setting background-color of an input[type="search"] does not work
Summary: Setting background-color of an input[type="search"] does not work
Status: REOPENED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Forms (show other bugs)
Version: 525.x (Safari 3.2)
Hardware: All All
: P3 Normal
Assignee: Nobody
URL: http://jsbin.com/omiYUmi/1/edit
Keywords:
: 33636 45417 53479 (view as bug list)
Depends on:
Blocks:
 
Reported: 2013-08-17 22:13 PDT by Emily
Modified: 2014-02-14 16:55 PST (History)
18 users (show)

See Also:


Attachments
Patch (3.86 KB, patch)
2013-10-11 11:31 PDT, Santosh Mahto
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from webkit-ews-02 for mac-mountainlion (471.06 KB, application/zip)
2013-10-11 12:39 PDT, Build Bot
no flags Details
Archive of layout-test-results from webkit-ews-11 for mac-mountainlion-wk2 (457.37 KB, application/zip)
2013-10-11 13:19 PDT, Build Bot
no flags Details
Archive of layout-test-results from webkit-ews-07 for mac-mountainlion (472.81 KB, application/zip)
2013-10-11 13:43 PDT, Build Bot
no flags Details
Patch (5.51 KB, patch)
2013-10-14 06:45 PDT, Santosh Mahto
no flags Details | Formatted Diff | Diff
Patch (5.78 KB, patch)
2013-10-14 11:47 PDT, Santosh Mahto
no flags Details | Formatted Diff | Diff
Patch (5.91 KB, patch)
2013-10-14 20:28 PDT, Santosh Mahto
no flags Details | Formatted Diff | Diff
Patch (17.63 KB, patch)
2013-10-14 21:43 PDT, Santosh Mahto
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Emily 2013-08-17 22:13:21 PDT
in safari ,the background-color of input[type="search"] can't work
Comment 1 Santosh Mahto 2013-10-11 11:31:11 PDT
Created attachment 214006 [details]
Patch
Comment 2 Build Bot 2013-10-11 12:39:52 PDT
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
Comment 3 Build Bot 2013-10-11 12:39:53 PDT
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 4 Build Bot 2013-10-11 13:18:59 PDT
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
Comment 5 Build Bot 2013-10-11 13:19:01 PDT
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 6 Build Bot 2013-10-11 13:43:51 PDT
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
Comment 7 Build Bot 2013-10-11 13:43:53 PDT
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
Comment 8 Santosh Mahto 2013-10-14 06:45:59 PDT
Created attachment 214149 [details]
Patch
Comment 9 Ryosuke Niwa 2013-10-14 09:49:09 PDT
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."
Comment 10 Darin Adler 2013-10-14 10:15:54 PDT
Was this done intentionally? Would background colors make some search inputs unreadable?
Comment 11 Ryosuke Niwa 2013-10-14 10:39:44 PDT
According to http://trac.webkit.org/browser/trunk/WebCore/rendering/RenderTheme.cpp?rev=18248, this wasn't intentional.
Comment 12 Ryosuke Niwa 2013-10-14 10:46:43 PDT
Also, we should probably have a pixel test for this.
Comment 13 Santosh Mahto 2013-10-14 11:47:26 PDT
Created attachment 214173 [details]
Patch
Comment 14 Ryosuke Niwa 2013-10-14 12:14:14 PDT
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.
Comment 15 Santosh Mahto 2013-10-14 20:28:22 PDT
Created attachment 214224 [details]
Patch
Comment 16 Ryosuke Niwa 2013-10-14 20:41:29 PDT
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.
Comment 17 Alexey Proskuryakov 2013-10-14 21:21:03 PDT
I'm confused. How can there be an expected.png for a test only text?
Comment 18 Ryosuke Niwa 2013-10-14 21:21:58 PDT
(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.
Comment 19 Santosh Mahto 2013-10-14 21:43:02 PDT
Created attachment 214232 [details]
Patch
Comment 20 WebKit Commit Bot 2013-10-14 22:54:05 PDT
Comment on attachment 214232 [details]
Patch

Clearing flags on attachment: 214232

Committed r157443: <http://trac.webkit.org/changeset/157443>
Comment 21 WebKit Commit Bot 2013-10-14 22:54:09 PDT
All reviewed patches have been landed.  Closing bug.
Comment 22 gur.trio 2013-12-20 16:49:05 PST
*** Bug 33636 has been marked as a duplicate of this bug. ***
Comment 23 gur.trio 2013-12-23 20:31:12 PST
*** Bug 45417 has been marked as a duplicate of this bug. ***
Comment 24 gur.trio 2013-12-26 00:57:11 PST
*** Bug 53479 has been marked as a duplicate of this bug. ***
Comment 25 mitz 2013-12-30 10:15:45 PST
(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!
Comment 26 mitz 2014-02-14 16:55:38 PST
I reverted the incorrect change in r164145.