Unlike other types of input elements (e.g. type="text"), input elements with type="search" don’t show a visible default focus outline. The default focus outline is an essential accessibility feature and should be always visible. <rdar://problem/19398333>
Created attachment 244388 [details] patch
Comment on attachment 244388 [details] patch Attachment 244388 [details] did not pass mac-ews (mac): Output: http://webkit-queues.appspot.com/results/5557827493953536 New failing tests: fast/css/focus-ring-exists-for-search-field.html
Created attachment 244391 [details] Archive of layout-test-results from ews100 for mac-mountainlion The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: ews100 Port: mac-mountainlion Platform: Mac OS X 10.8.5
Created attachment 244392 [details] patch
Comment on attachment 244392 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=244392&action=review I’m going to say review+ but I have two substantial concerns: This seems like a pretty big deal and would change how websites look; we’ve had this behaving like this for a long time. Do we have some examples of websites we looked at to vet this change not only preserves this feature to satisfy accessibility needs but also works well and looks good in the actual websites? I could even imagine contexts in apps or widgets where someone might have wanted to turn this off. > Source/WebCore/rendering/RenderThemeMac.mm:1605 > + if (isFocused(o) && o.style().outlineStyleIsAuto()) > + wkDrawCellFocusRingWithFrameAtTime(search, NSRect(unzoomedRect), documentView, std::numeric_limits<double>::max()); RenderThemeMac::paintMenuList calls setFocusedElementNeedsRepaint. Is there a reason that is correct there but is not needed here?
(In reply to comment #5) > Comment on attachment 244392 [details] > patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=244392&action=review > > I’m going to say review+ but I have two substantial concerns: > > This seems like a pretty big deal and would change how websites look; we’ve > had this behaving like this for a long time. Do we have some examples of > websites we looked at to vet this change not only preserves this feature to > satisfy accessibility needs but also works well and looks good in the actual > websites? I could even imagine contexts in apps or widgets where someone > might have wanted to turn this off. I'll do some browsing and post screenshots here of what it looks like > > > Source/WebCore/rendering/RenderThemeMac.mm:1605 > > + if (isFocused(o) && o.style().outlineStyleIsAuto()) > > + wkDrawCellFocusRingWithFrameAtTime(search, NSRect(unzoomedRect), documentView, std::numeric_limits<double>::max()); > > RenderThemeMac::paintMenuList calls setFocusedElementNeedsRepaint. Is there > a reason that is correct there but is not needed here? I'll have to investigate what that does and why it did not seem necessary here
> > > > > Source/WebCore/rendering/RenderThemeMac.mm:1605 > > > + if (isFocused(o) && o.style().outlineStyleIsAuto()) > > > + wkDrawCellFocusRingWithFrameAtTime(search, NSRect(unzoomedRect), documentView, std::numeric_limits<double>::max()); > > > > RenderThemeMac::paintMenuList calls setFocusedElementNeedsRepaint. Is there > > a reason that is correct there but is not needed here? > > I'll have to investigate what that does and why it did not seem necessary > here It looks like we probably need that NeedsRepaint, because of the comments in [Mac] Allow focus rings to redraw themselves if necessary https://bugs.webkit.org/show_bug.cgi?id=132593 "Mac allows focus rings to change rendering over time. Expose this functionality to WebKit by having the rendering code detect that a focused element has asked to repaint itself, and also have the FocusController object remember how long it has been since an element was focused."
Created attachment 246085 [details] screenshot1
Created attachment 246086 [details] screenshot2
Created attachment 246087 [details] screenshot3
Created attachment 246088 [details] screenshot4
Created attachment 246089 [details] screenshot5
http://trac.webkit.org/changeset/179796
This test fails on all Mavericks bots: https://webkit-test-results.appspot.com/dashboards/flakiness_dashboard.html#showAllRuns=true&tests=fast%2Fcss%2Ffocus-ring-exists-for-search-field.html Looks like a font difference, so I'm going to land platform specific results.