UNCONFIRMED 66937
Padding size of input type search element is not locked on Webkit-GTK 
https://bugs.webkit.org/show_bug.cgi?id=66937
Summary Padding size of input type search element is not locked on Webkit-GTK
Sameer Patil
Reported 2011-08-25 07:15:36 PDT
Created attachment 105179 [details] Padding test for input type search element Padding size of input type element is not locked on Webkit-GTK as like Safari. Please check attached test page and screen shot comparing Safari and Webkit-GTK behavior. Here padding of input element type search is locked on Safari, same is not observed on Webkit-GTK. Tested on: Windows Safari (5.1):Passed Windows IE(6.0):Passed Linux Mozilla Firefox(6.0):Failed Linux Opera(11.50):Failed
Attachments
Padding test for input type search element (659 bytes, text/html)
2011-08-25 07:15 PDT, Sameer Patil 		no flags
Details
Safari and Webkit GTK comparison (30.50 KB, image/jpeg)
2011-08-25 07:16 PDT, Sameer Patil 		no flags
Details
Patch (4.44 KB, patch)
2011-08-25 07:25 PDT, Sameer Patil 		no flags
DetailsFormatted DiffDiff
Patch (5.99 KB, patch)
2011-09-06 06:39 PDT, Sameer Patil 		gustavo: review-
webkit.review.bot: commit-queue-
DetailsFormatted DiffDiff
Focus ring comparison (with and without setting outline offset when input search is active) (41.50 KB, image/jpeg)
2011-09-06 07:01 PDT, Sameer Patil 		no flags
Details
Input and Input search focus ring test (569 bytes, text/html)
2011-09-13 05:09 PDT, Sameer Patil 		no flags
Details
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Sameer Patil
Comment 1 2011-08-25 07:16:24 PDT
Created attachment 105180 [details] Safari and Webkit GTK comparison
Sameer Patil
Comment 2 2011-08-25 07:25:26 PDT
Created attachment 105182 [details] Patch
Martin Robinson
Comment 3 2011-08-30 09:35:48 PDT
Comment on attachment 105182 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=105182&action=review > Source/WebCore/ChangeLog:11 > + * platform/gtk/RenderThemeGtk.cpp: > + (WebCore::RenderThemeGtk::adjustSearchFieldStyle): Please fill these in. > Source/WebCore/platform/gtk/RenderThemeGtk.cpp:375 > + if (e && e->focused() && e->document()->frame()->selection()->isFocusedAndActive()) > + style->setOutlineOffset(-2); I don't see this on other platforms at all. It seems unrelated. Why is it here? > LayoutTests/fast/forms/search-styled-padding.html:15 > +<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Strict//EN" > + "http://www.w3.org/TR/xhtml1/DTD/xhtml1-strict.dtd"> > +<html xmlns="http://www.w3.org/1999/xhtml" xml:lang="en" lang="en"> > +<head> > +<style> > +.inputtext, > +.inputpassword{border:1px solid #bdc7d8;font-family:"lucida grande", tahoma, verdana, arial, sans-serif;font-size:11px;padding:3px;} > +.inputsearch{background:white url(./resources/apple.gif) no-repeat 3px 4px;padding-left:17px;padding-bottom: 100px;} > +</style> > + > +</head> > +<body> > +<input type="search" class="inputtext inputsearch" title="Search for Events" placeholder="Search for Events" id="q_dashboard" name="q" value="" results="10" /> > +</body> > +</html> It seems like this test case could be greatly simplified. Should a text input type="search" field with some padding suffice? Is this covered by any existing tests? Be sure to generate your results with HEAD, as we've switched to GTK+ 3.x now.
Sameer Patil
Comment 4 2011-09-06 06:39:55 PDT
Created attachment 106411 [details] Patch
Sameer Patil
Comment 5 2011-09-06 06:58:03 PDT
Comment on attachment 105182 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=105182&action=review >> Source/WebCore/ChangeLog:11 >> + (WebCore::RenderThemeGtk::adjustSearchFieldStyle): > > Please fill these in. Added details regarding change. >> Source/WebCore/platform/gtk/RenderThemeGtk.cpp:375 >> + style->setOutlineOffset(-2); > > I don't see this on other platforms at all. It seems unrelated. Why is it here? This has been done for focusring. It has been there on windows platform(WebCore/rendering/RenderThemeWin.cpp). If focus ring is enabled and if we do active input type search element by clicking it then focus ring will look outset. Please check screenshot showing comparison of focusring behavior when we do and don't set outline offset on input type search element. >> LayoutTests/fast/forms/search-styled-padding.html:15 >> +</html> > > It seems like this test case could be greatly simplified. Should a text input type="search" field with some padding suffice? Is this covered by any existing tests? Be sure to generate your results with HEAD, as we've switched to GTK+ 3.x now. I have added simplified Layout test for input type="search" element. All the parameters(padding size) of test is not covered by single test for input type search elements so added one. Yes I have generated the test results with the head.
Sameer Patil
Comment 6 2011-09-06 07:01:20 PDT
Created attachment 106413 [details] Focus ring comparison (with and without setting outline offset when input search is active)
WebKit Review Bot
Comment 7 2011-09-06 07:58:33 PDT
Comment on attachment 106411 [details] Patch Attachment 106411 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/9591872
Martin Robinson
Comment 8 2011-09-11 08:53:49 PDT
(In reply to comment #6) > Created an attachment (id=106413) [details] > Focus ring comparison (with and without setting outline offset when input search is active) Your GTK+ theme is odd, so I can't really parse it right away. Is the red line the focus ring? Why do we want the focus ring to overlap the widget itself. GTK+ themes specify where the focus ring goes, why shouldn't we obey them?
Sameer Patil
Comment 9 2011-09-13 05:07:18 PDT
(In reply to comment #8) > (In reply to comment #6) > > Created an attachment (id=106413) [details] [details] > > Focus ring comparison (with and without setting outline offset when input search is active) > > Your GTK+ theme is odd, so I can't really parse it right away. Is the red line the focus ring? Why do we want the focus ring to overlap the widget itself. GTK+ themes specify where the focus ring goes, why shouldn't we obey them? Yes red line is focus ring, which i set through html.css to demonstrate. Focus ring set through outline property from html.css. I am attaching html page showing comparison of focus ring for input and input search element. I had set the outline(color,width) in css in same html page to simulate focus ring active state for input and input type search element.I didn't get where the gtk+ theme rules have been defined for focus ring on input or any focusable element? Do you mean any specific css file or RenderThemeGtk?
Sameer Patil
Comment 10 2011-09-13 05:09:55 PDT
Created attachment 107166 [details] Input and Input search focus ring test
Martin Robinson
Comment 11 2011-09-14 08:48:25 PDT
(In reply to comment #10) > Yes red line is focus ring, which i set through html.css to demonstrate. Focus ring set through outline property from html.css. I am attaching html page showing comparison of focus ring for input and input search element. I had set the outline(color,width) in css in same html page to simulate focus ring active state for input and input type search element.I didn't get where the gtk+ theme rules have been defined for focus ring on input or any focusable element? Do you mean any specific css file or RenderThemeGtk? Take a look at RenderThemeGtk3.cpp. focus-line-width and focus-padding affect how focus rings appear. Please ensure that your changes do not negatively affect theming. Does WebCore take over drawing focus rings in some CSS theming situations?
Gustavo Noronha (kov)
Comment 12 2013-10-11 07:54:14 PDT
Comment on attachment 106411 [details] Patch Been a while since a review was done asking for some things to be fixed/ensured, I'm setting r- for now, please send an updated patch if it still makes sense.
Note You need to log in before you can comment on or make changes to this bug.