Summary: When you set outline:auto on a div, it shows the "halo" outline around the elements *inside* the div, rather than the div itself To reproduce: see attached test case. Expected results: We should see a blue halo around the bottom-most rectangle, just like we see a solid blue halo around the div just above it. We should not see the outline around the contained text or image. Actual results: we see a halo around the text elements, and around the image (sometimes clipped), but not around the div itself. Regression: Nothing formal here, but I recall on older webkits (pre-10.4.2), I couldn't get the halo to show up at all if the div wasn't "focused".
Created attachment 2945 [details] HTML file showing problem with outline auto Note: I'm now noticing that I am not seeing the "halo" outline around the elements until you select the elements by dragging the mouse over them.
I'm using 10.3.9. The outline shows as you state (it doesn't need to be in focus). However I'm not totally sure what correct behaviour is as no other browser I tested render it at all. outline: auto is a css 3 property so other browsers have not implemented it yet. Opera and IE Mac show outlines for the css 2 properties, but Gecko browsers do not show them at all. The css spec states "in CSS3, 'outline-style' accepts the value 'auto'. The 'auto' value permits the user agent to render a custom outline style, typically a style which is either a user interface default for the platform, or perhaps a style that is richer than can be described in detail in CSS, e.g. a rounded edge outline with semi-translucent outer pixels that appears to glow. As such, this specification does not define how the 'outline-color' is incorporated or used (if at all) when rendering 'auto' style outlines. User agents may treat 'auto' as 'solid'." It seems to not state what the correct rendering is, but I'd expect that it should render the same as other styles, just in an OS X look and feel, instead of a flat colour for example. So suspect it is a bug. I'll attach a further reduced test case with just a div that has outline-style: auto; applied to it.
Created attachment 2995 [details] further reduced testcase
First testcase is good, second testcase doesn't match firefox/mac behaviour at all (ffx gives no outline). Confirmed in current ToT
Created attachment 4894 [details] Patch that seems to fix the two testcases Here is a patch that draws the outlines in these two cases. How ever, since it's so simple and reduces "functionality", I'm not sure at all that it will fix the issue at hand fully and without side effects. However, somebody who wants to actually fix the bug (or is interested in the possible bounty) can perhaps make use of the information in this patch.
The intent behind my implementation of focus ring outline was to encompass overflowed elements, producing irregular shapes that really fit around all content (even content that sticks out of the block). I think the real bug is in RenderFlow::addFocusRingRects, namely that the block's rect is never added to the set of rects as well. You just need to call the base class if you're a block I think. This completely bogus code: if (element() && element()->isContentEditable()) { if (element()->parentNode() && !element()->parentNode()->isContentEditable() && !element()- >hasTagName(bodyTag)) p->addFocusRingRect(_tx, _ty, width(), height()); return; } should be removed as well. I have no idea who put that there or why. It makes no sense.
Created attachment 4922 [details] reduced testcase for non-rectangular outlines A further testcase to demonstrate the behaviour of an outline when it should be non-rectangular. Currently, both 'auto' and styled outlines are rectangular.
Created attachment 4923 [details] patch This patch fixes addFocusRingRects to include the outermost rect in the focus ring. This has the effect of causing 'auto'-styled outlines to be non-rectangular, as in the previous testcase. However, styled outlines are still rectangular, and follow the outer-most element.
Comment on attachment 4923 [details] patch Dave should review this.
Comment on attachment 4923 [details] patch You definitely only want to add in the focus ring rects for block flows and not for inlines (RenderFlow is the base class of both).
Created attachment 4931 [details] patch 2 Moved the addFocusRingRects function to RenderBlock.
Comment on attachment 4931 [details] patch 2 I just meant that the new code only needed to be done for RenderBlocks. You want to still call through to the base class for the rest of the code (which should be in RenderFlow still and executed by both inlines and blocks).
Created attachment 4954 [details] patch 3 Oops. I misunderstood. I added an if (isRenderBlock()) to patch 1.
Comment on attachment 4954 [details] patch 3 I tried entering 'hyatt' as the requestee, but bugzilla just tells me that 'hyatt' doesn't match anything...
Comment on attachment 4954 [details] patch 3 Need to enter "hyatt@apple.com" as the requestee.
Comment on attachment 4954 [details] patch 3 r=me
We should probably add some code to prevent the outlines from being drawn on focus for the body element. This could be done in html4.css. Instead of *:focus { outline ... } I think: :not(html):not(body):focus { outline ... } would work.
Dave, html4.css already has this: :focus { outline: auto 3px #1f5ccf } html:focus, body:focus { outline: none }