Bug 25701 - REGRESSION(r38788 & r42020): styled searchfields look wrong on Windows, affects Facebook
Summary: REGRESSION(r38788 & r42020): styled searchfields look wrong on Windows, affec...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Forms (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC Windows Vista
: P2 Normal
Assignee: Alice Liu
URL: http://web.me.com/alice.liu/FacebookE...
Keywords: InRadar, PlatformOnly, Regression
Depends on:
Blocks:
 
Reported: 2009-05-11 12:48 PDT by Alice Liu
Modified: 2009-06-23 17:24 PDT (History)
3 users (show)

See Also:


Attachments
screenshot (6.40 KB, image/png)
2009-05-11 12:49 PDT, Alice Liu
no flags Details
proposed patch without testcase (1.12 KB, patch)
2009-05-11 12:50 PDT, Alice Liu
hyatt: review-
Details | Formatted Diff | Diff
patch with test case (6.54 KB, patch)
2009-05-11 16:02 PDT, Alice Liu
mitz: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Alice Liu 2009-05-11 12:48:47 PDT
On Windows, now that search fields have -webkit-appearance: textfield and the code path to adjust styling for searchfields has been replaced by rules in a user agent style sheet, styled search fields can look weird in Safari on Windows. 

originally found at http://www.facebook.com/events.php?ref=sb, the search field now has both the developer-specified style plus the webkit search field decorations, when it should just have the webkit search field decoration.

reduction at http://web.me.com/alice.liu/FacebookEvents.html

<rdar://problem/6855916>
Comment 1 Alice Liu 2009-05-11 12:49:24 PDT
Created attachment 30198 [details]
screenshot
Comment 2 Alice Liu 2009-05-11 12:50:28 PDT
Created attachment 30199 [details]
proposed patch without testcase
Comment 3 Dave Hyatt 2009-05-11 13:00:01 PDT
Comment on attachment 30199 [details]
proposed patch without testcase

I don't think you intended to prevent the specification of padding on text fields.  I'm pretty sure browsers allow the padding of text fields to be customized.

It's not clear to me why we can't support customizable padding on search fields, so I don't really understand why we'd prevent that?

I believe you can just remove the rule for input[type="search"] to get -webkit-appearance: searchfield, right?  I assume that's specified in the base CSS file already.
Comment 4 Alice Liu 2009-05-11 13:21:04 PDT
=\  I forgot to explain the padding thing fully. 

http://trac.webkit.org/changeset/42020

this change removed the code path that adjusted the appearance of searchfield after all the style was already applied.  We used to call RenderThemeWin::adjustSearchFieldStyle, and now we just specify the padding rules in themeWin.css, but the problem was that the padding rules needed to be applied last in order to look exactly the way it used to.  

Do we no longer care that much to preserve this padding we had in Safari 3?  If not, i can take it out.  I think as long as we just make the change to not respect background image styling on searchfields, then the facebook searchfield will look okay on Windows.  
Comment 5 mitz 2009-05-11 13:28:49 PDT
(In reply to comment #3)
> It's not clear to me why we can't support customizable padding on search
> fields, so I don't really understand why we'd prevent that?

In the facebook example, if you allowed them to set padding, the search field's own magnifying glass will be offset to far to the right. I think they specify padding in order for the text to not overlap the magnifying glass that comes from the background-image. If WebKit ignores the background-image and supplies its own magnifying glass, then it had better ignore the padding as well.
Comment 6 Dave Hyatt 2009-05-11 14:17:44 PDT
Yeah, sounds like we're just stuck being rigidly uncustomizable like Mac just for compatibility.

The padding needs to be locked from code rather than in the stylesheet though if we do this, since you should be able to customize padding when -webkit-appearance is turned off for input[type=search].

Comment 7 Alice Liu 2009-05-11 16:02:21 PDT
Created attachment 30206 [details]
patch with test case
Comment 8 Adam Roben (:aroben) 2009-05-11 16:25:48 PDT
Comment on attachment 30206 [details]
patch with test case

Maybe we should add a comment to adjustSearchFieldStyle saying why overriding the padding is important (probably referencing this bug)?
Comment 9 Eric Seidel (no email) 2009-06-23 17:22:31 PDT
Did this ever land?
Comment 10 mitz 2009-06-23 17:24:57 PDT
Fixed by Alice in <http://trac.webkit.org/changeset/43522>.