Bug 119967 – Setting background-color of an input[type="search"] does not work

• Component: Forms
• Status: REOPENED

[Emily]

in safari ,the background-color of input[type="search"] can't work

[Santosh Mahto]

Created attachment 214006 [details]
Patch

[Build Bot]

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

[Build Bot]

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

[Build Bot]

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

[Build Bot]

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

[Build Bot]

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

[Build Bot]

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

[Santosh Mahto]

Created attachment 214149 [details]
Patch

[Ryosuke Niwa]

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."

[Darin Adler]

Was this done intentionally? Would background colors make some search inputs unreadable?

[Ryosuke Niwa]

According to http://trac.webkit.org/browser/trunk/WebCore/rendering/RenderTheme.cpp?rev=18248, this wasn't intentional.

[Ryosuke Niwa]

Also, we should probably have a pixel test for this.

[Santosh Mahto]

Created attachment 214173 [details]
Patch

[Ryosuke Niwa]

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.

[Santosh Mahto]

Created attachment 214224 [details]
Patch

[Ryosuke Niwa]

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.

[Alexey Proskuryakov]

I'm confused. How can there be an expected.png for a test only text?

[Ryosuke Niwa]

(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.

[Santosh Mahto]

Created attachment 214232 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 214232 [details]
Patch

Clearing flags on attachment: 214232

Committed r157443: <http://trac.webkit.org/changeset/157443>

[WebKit Commit Bot]

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.