Starting with Safary 1.3/2.0, I found that the CSS outline command appears to be applied twice when used on a list element. I'm using a hover command on the <ul> <li> element, and it used to cause the list entry to be highlighted with a outline when the mouse was over it. Starting with 1.3/2.0, now the outline is still there, but there is a 2nd, inner outline right around the text entry itself? This bug still seems to be there in the latest build I tried today. thanks, Hanspeter Schaub
Could you please post a public URL or attach the "conferences.html" file to this bug - we are unable to see what's on your private iDisk !
I have reduced the page to a simple text case that reproduces the problem. The test case contains a simple ordered list uses class='test': <ol class="test"> <li> OL element with a single child LI element </li> </ol> A CSS rule is applied to the OL: ol.test li:hover { outline: dashed; padding:0.5em; } The problem occurs when padding is specified (padding: 0.5em) If you open the test case "outline_sample.html" and hover over the OL, both the OL and LI appear with a dashed outline. If padding is removed, this issue will no longer reproduce. Under Firefox 1.0.4 , this test case is rendered with just the outline for the OL.
Created attachment 2167 [details] Reduced test case
Created attachment 2188 [details] simple test case to see "outline" error with lists
Created attachment 2463 [details] proposed patch I found that the code in renderflow.cpp was causing the erroneous outline to be drawn on the list elements. In this patch I simply commented out the old code. Please not, I'm not a professional programmer, but an engineer :-) I found this fix by searching through the code for anything related to drawing outlines and testing those code sections. I did run the regression test and the modified code passes all the test.
Created attachment 2464 [details] list outline regression test This is the regression test for the proposed patch. It shows a simple unordered list with the outline applied to the list element. If the test is successful, then a single outline should appear over the list element. If the browser fails, then two outlines are visible.
Created attachment 2474 [details] Actual behavior
Created attachment 2475 [details] Expected behavior Expected behavior (seen after patch in Attachment 2463 [details] is applied).
After looking into this bug, it appears that the outline property is being applied to both the <li> element as well as its enclosing text. Updated title to reflect this.
See also Bug 3611 for another outline property issue.
Comment on attachment 2463 [details] proposed patch This patch is incorrect and will break all non-auto outline styles on inlines. The real bug occurs in render_line.cpp around line 754. Root lines should not be adding their block objects to the outlineObjects dict.
Created attachment 2559 [details] fixes the issue where the outline is applied to the list text element Following Dave Hyatt's suggestion, I added a check to render_line.cpp (line 754) to check that the object is not a root object. The previously supplied regression test case renders properly, and running run-webkit-tests passes all previous tests.
+ is not what you should set. ? is how you ask for review. + means the patch received review.
Comment on attachment 2559 [details] fixes the issue where the outline is applied to the list text element Dave Hyatt should be the one to review this.
Comment on attachment 2559 [details] fixes the issue where the outline is applied to the list text element Style is wrong. Make sure to remove the extra space between the two close parentheses. Other than that, r=me.
Created attachment 2661 [details] minor revision to patch, removed space between closing parentheses fixes a small style error in the earlier patch. The space between the two closing parentheses has been removed in this version.
Comment on attachment 2661 [details] minor revision to patch, removed space between closing parentheses r=me