Created attachment 41465 [details] Screen shot of mis-rendered search box (cross post from http://code.google.com/p/chromium/issues/detail?id=15665) Chrome Version : 3.0.182.3 (Offizieller Build 17055) URLs (if applicable) : all Apple websites Other browsers tested: yes Add OK or FAIL after other browsers where you have tested this issue: Safari 4:OK Firefox 3.x:OK IE 7:OK IE 8:OK What steps will reproduce the problem? 1.visit any Apple website and start typing something into the search box in the upper right hand corner What is the expected result? - Searchbox should be displayed properly and should suggest items as you type. What happens instead? - searchbox not displayed properly and no suggestions appear.
Created attachment 41467 [details] Proposed fix - implement rounded border rendering on the text field painter Tested by running in side a Chromium build. The search box background color is still grey on www.apple.com, but this is in line with what the site's css is setting.
Comment on attachment 41467 [details] Proposed fix - implement rounded border rendering on the text field painter 8 No new tests: this is chromium only code. Does not mean it doesn't need testing! Eventually Chromium's results will be in webkit.org and thus we won't have this issue at all. For now, if this is covered by existing tsts, please list those tests. If it is not, then we need to add a new test and check in results for the other platforms.
Created attachment 41633 [details] Prosed fix, with test
Comment on attachment 41633 [details] Prosed fix, with test Tabs in the ChangeLog will cause the commit to fail.
Created attachment 41671 [details] Identical to previous patch, but with tabs removed from ChangeLog Many thanks! Not even sure how the tabs got in there...
Created attachment 41672 [details] Proposed fix (with tabs removed) Agh didn't click the patch tick box, lets try that again...
Comment on attachment 41672 [details] Proposed fix (with tabs removed) Looks like you've added results only to platform/mac. Either we need to have results in a shared location (right next to the test) or we'll need results for all the other platforms. It looks like this is a rendering change, so it can't be dumpAsText, so we may end up needing results for all the other platforms. :( Sadly we don't have an easy way to do that, although it is possible to check-in and ask nicely for others to add the results for you, or grab the new results off of the buildbots after your checkin.
Created attachment 41770 [details] Proposed fix + test, with -expected files in global (not platform specific) location Eric suggested moving the expected files to the global location, and adding other platforms in if we pickup differences on the try bots
Comment on attachment 41770 [details] Proposed fix + test, with -expected files in global (not platform specific) location Looks fine to me. This shouldn't be landed by the queue though since it's expected to cause failures on other platforms.
Comment on attachment 41770 [details] Proposed fix + test, with -expected files in global (not platform specific) location Joth is going to watch it land.
Seems like a bad idea given how backed up the queue is right now: http://webkit-commit-queue.appspot.com/ (mostly due to the tree being red all day). Also since this is expected to fail, a committer should be around to clean up the mess. Does joth have commit bit? And if so, why wouldnt' he just commit this himself? I recommend cq-ing this.
The commit-queue is super-useful for set-and-forget patches, where they are expected to be commited w/o any issue, and letting the bot do it later makes perfect sense. cq isn't very good for patches which require committer oversight. :(
It's not clear to me what webkit failures are expected? Dimitri thought that only safari-mac expected output is required in the webkit submit (which this patch has) so it should go through fine. There will be a follow on step to clean up the chromium tree after this lands, but that's beyond the scope of this commit. Point taken about the webkit tree being red all day though :-(
Comment on attachment 41770 [details] Proposed fix + test, with -expected files in global (not platform specific) location Rejecting patch 41770 from commit-queue. Failed to run "['git', 'svn', 'dcommit']" exit_code: 1 Last 500 characters of output: Log M WebCore/rendering/RenderThemeChromiumWin.cpp A repository hook failed: MERGE request failed on '/repository/webkit/trunk': Commit blocked by pre-commit hook (exit code 1) with output: The following files contain tab characters: trunk/LayoutTests/fast/css/text-input-with-webkit-border-radius.html Please use spaces instead to indent. If you must commit a file with tabs, use svn propset to set the "allow-tabs" property. at /usr/local/libexec/git-core//git-svn line 469
Created attachment 42046 [details] Fix previous patch which failed (tabs in the .html file) Is there a pre-submit lint tool for patches?
check-webkit-style does a minor amount of linting. It's derived from Google's cpp_lint. It needs to be expanded to deal with things like mising ChangeLogs and tabs though. :( You should feel encouraged to file bugs about us having better linting tools.
Comment on attachment 42046 [details] Fix previous patch which failed (tabs in the .html file) Clearing flags on attachment: 42046 Committed r50237: <http://trac.webkit.org/changeset/50237>
All reviewed patches have been landed. Closing bug.
Created attachment 42080 [details] Add tests to skip list for platforms that do not yet have expected output
Seems it would be easier to just get the results from the bots and check those in, rather than skipping. or?
Comment on attachment 42080 [details] Add tests to skip list for platforms that do not yet have expected output r- for no changelog and Eric's comment above: https://bugs.webkit.org/show_bug.cgi?id=30547#c20
Created attachment 42160 [details] Add expected output for GTK & QT (as created by build bots)
Comment on attachment 42160 [details] Add expected output for GTK & QT (as created by build bots) Needs a ChangeLog. :) Also we'll need to re-open the bug if we want things like the commit-queue to notice the patches. :) I'll make sure you have edit bug privs if you dont' already.
Re-opening in order to add patch to fixup GTK & QT tests. Will add ChangleLog to the patch momentarily. Thanks!
Created attachment 42164 [details] Add expected output for GTK & QT (this time with ChangeLog) I'll get the hang of this eventually! Thanks.
Comment on attachment 42164 [details] Add expected output for GTK & QT (this time with ChangeLog) Lovely!
Comment on attachment 42164 [details] Add expected output for GTK & QT (this time with ChangeLog) Clearing flags on attachment: 42164 Committed r50314: <http://trac.webkit.org/changeset/50314>