In LayoutTests/fast/forms/search-transformed.html, Chromium renders decoration of "search" input field (loupe icon and close button) at wrong position. This is caused due to a bug in WebCore/rendering/RenderThemeChromiumSkia.cpp. Chromium bug: Layout Test failure: type=search input has disappearing (x) http://code.google.com/p/chromium/issues/detail?id=20439
Created attachment 40933 [details] Fix decoration position in search input field.
FYI: Patch and try result can also be viewed from: http://codereview.chromium.org/269030/show
Comment on attachment 40933 [details] Fix decoration position in search input field. Where can I see the resulting rendered image of the new search box? Style: 154 IntRect calcParentBoxOfSearchFieldItem(RenderObject* o, const IntRect& r); calc is a strange abbreviation. Normally we use full english words, although "calc" might be OK. Again, full words are generally preferred over letters: 392 IntRect bounds = r; You're trying to get the content rect of the parent box in the grandparent's coordinates? I feel like there is an easier way to do that...
Created attachment 41087 [details] Screenshot of current and fixed rendering
(In reply to comment #3) > (From update of attachment 40933 [details]) > Where can I see the resulting rendered image of the new search box? Uploaded. > > Style: > 154 IntRect calcParentBoxOfSearchFieldItem(RenderObject* o, const > IntRect& r); > > calc is a strange abbreviation. Normally we use full english words, although > "calc" might be OK. > Sure. I'll update the patch accordingly. > Again, full words are generally preferred over letters: > 392 IntRect bounds = r; > > > You're trying to get the content rect of the parent box in the grandparent's > coordinates? I feel like there is an easier way to do that... We need to find the *drawing position* of parent box. Since the coordinate is relative to the current layer (created by -webkit-transform), it's not same as the absolute position. I tried to find more adequate way to calculate this but I could not. Suggestions are welcome.
Created attachment 41262 [details] Fix decoration position in search input field. (Fixed function name)
This patch seems reasonable, but it involves drawing to the screen, which I know nothing about. Eric, would you be willing to have another look?
You may already be familiar with this, but this is the box model to which I refer: http://www.w3.org/TR/CSS21/box.html Most of those accessors are available on RenderObject or RenderBoxModelObject. I still don't really like r and o for names. Particularly r, because it doesn't tell me anythign about what coordinate system the rect your passing in should be in. damageRectInLocalCoords would be a much better name if it happens to be the damage rect in content box coordinates relative to "o". I'd have to stare harder to figure out what you're actually trying to do here.
Thanks, I'll try to make another patch. Dropping r? flag.
Created attachment 47502 [details] Fix decoration position in search input field. (v3: Totally rewritten)
Comment on attachment 47502 [details] Fix decoration position in search input field. (v3: Totally rewritten) I don't understand why you're adding all the const declarations? That's unrelated to this patch and should not be part of this. WebKit seems to have mixed feelings on the value of const anyway.
(In reply to comment #11) > (From update of attachment 47502 [details]) > I don't understand why you're adding all the const declarations? That's > unrelated to this patch and should not be part of this. WebKit seems to have > mixed feelings on the value of const anyway. It's because the first argument of RenderThemeChromiumSkia::convertToPaintingRect is marked as const. The signature of this function is originally taken from RenderThemeMac::convertToPaintingRect, whose first argument is also marked as const. I can certainly strip away these consts, but, to be honest, I really think there is no reason to leave the arguments of offsetContainer non-const. I believe it's common and important in C++ to distinguish const pointer variables from non-const ones.
Created attachment 48005 [details] Fix decoration position in search input field. (v4: removed consts)
Comment on attachment 48005 [details] Fix decoration position in search input field. (v4: removed consts) OK.
(In reply to comment #14) > (From update of attachment 48005 [details]) > OK. Thank you for your review!
Comment on attachment 48005 [details] Fix decoration position in search input field. (v4: removed consts) Clearing flags on attachment: 48005 Committed r55617: <http://trac.webkit.org/changeset/55617>
All reviewed patches have been landed. Closing bug.