Summary: | outline:auto improperly puts outline around contained elements, not the actual div | ||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Dan Wood <dwood> | ||||||||||||||||
Component: | CSS | Assignee: | Dave Hyatt <hyatt> | ||||||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||||||
Severity: | Normal | CC: | ttalbot | ||||||||||||||||
Priority: | P2 | ||||||||||||||||||
Version: | 420+ | ||||||||||||||||||
Hardware: | Mac | ||||||||||||||||||
OS: | OS X 10.4 | ||||||||||||||||||
Attachments: |
|
Description
Dan Wood
2005-07-13 10:28:53 PDT
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 } |