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 41671[details]
Identical to previous patch, but with tabs removed from ChangeLog
Many thanks! Not even sure how the tabs got in there...
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
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 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.
2009-10-19 17:44 PDT, Jonathan Dixon
2009-10-19 17:53 PDT, Jonathan Dixon
2009-10-21 20:36 PDT, Jonathan Dixon
2009-10-22 11:42 PDT, Jonathan Dixon
2009-10-22 11:44 PDT, Jonathan Dixon
2009-10-23 18:24 PDT, Jonathan Dixon
commit-queue: commit-queue-
2009-10-28 11:53 PDT, Jonathan Dixon
2009-10-28 20:55 PDT, Jonathan Dixon
2009-10-29 17:12 PDT, Jonathan Dixon
2009-10-29 17:58 PDT, Jonathan Dixon