Closed Bug 328296 Opened 19 years ago Closed 18 years ago

Obscured outlines are drawn (through things that are on top and not transparent)

Categories

(Core :: Layout, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: jruderman, Assigned: roc)

References

()

Details

(Keywords: regression, testcase)

Attachments

(9 files)

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1) Gecko/20060219 Firefox/1.6a1 I first noticed this on Tinderbox: after clicking a star to see a "DHTML popup" message, I noticed that the star's outline was displayed on top of the message. I think this a regression from the Frame Display Lists patch (bug 317375).
Attached file testcase (deleted) —
Happens on Mac too.
OS: Windows XP → All
Hardware: PC → All
I think that's on purpose, see bug 317375, comment 82 and comment 83: me: " http://wargers.org/317375_painting_bug_absolute.html Painting bug with the patch. You should only see a small red and a small green box. Resizing the window fixes it. " Roc: " (In reply to comment #82) > http://wargers.org/317375_outline.htm > I see that outline is now always at the top, I guess that's on purpose, > right? Yes, that is recommended by the spec. "
Yes, I believe this is per spec.
Can you point me to the part of the spec that says that outlines should be on top of everything? I find it hard to believe because it seems weird and breaks any page that does stuff the way Tinderbox does.
Please. People. For the love of all things sweet and holy. Make your testcases trigger strict mode! It's not hard, just include "<!DOCTYPE HTML>" at the top of the page.
The answer to your question, btw, is CSS 2.1 Appendix E section 2 step 9.
Status: NEW → RESOLVED
Closed: 19 years ago
Resolution: --- → INVALID
Hixie, do you know why the spec says that? It seems wrong and broken to me.
*** Bug 341069 has been marked as a duplicate of this bug. ***
Attached file Testcase #2 (deleted) —
I agree with Jesse, the new behavior seems wrong and broken. CSS 2.1 specifies two alternatives for outline rendering, either at the end of step 7 (before positioned elements) or at step 10 (after positioned elements). http://www.w3.org/TR/CSS21/zindex.html#q2 Firefox 1.5/2, Opera 9 and Webkit have compatible implementations (step 7). IE7 does not implement outline as far as I can tell. CSS 2.1 recommends step 10 over earlier steps (without motivating why). The CSS 2.1 abstract says: <q>But most of all CSS 2.1 represents a "snapshot" of CSS usage: it consists of all CSS features that are implemented interoperably at the date of publication of the Recommendation.</q> Recommending step 10 is clearly in conflict with the goal set out in the abstract. It should recommend step 7 (if anything) because that's what been interoperably implemented.
IE most certainly does implement outline. I doubt they've removed it from IE7. Changing our behaviour is probably not hard but I won't change it unless Ian agrees the spec should be changed.
The primary use case for 'outline' is indicating focus. If one alternative is good for focus indication and another is good for some other mysterious use of 'outline', we should choose the first. For indicating focus, I think drawing visible outlines at the position of covered elements is better.
When the focused element is covered, it's usually because the site wants you to see what's on top. (At least, that's true in the Tinderbox example.)
(In reply to comment #12) > IE most certainly does implement outline. I doubt they've removed it from IE7. I tested again several examples in IE7 on Vista and XPSP2 and I couldn't find any example that worked. I even tried with vendor prefix. All the CSS compatibility charts I found agree with me: http://www.webdevout.net/browser-support#css http://en.wikipedia.org/wiki/Comparison_of_layout_engines_(CSS)#Properties http://www.quirksmode.org/css/contents.html http://aptana.com/reference/api/CSSElements.index-fields.html I understand that such charts are often wrong, incomplete and outdated, but in this case they agree. There are some information that IE for Mac (Tasman) has implemented 'outline', but that's not very interesting since 99.99% (my estimate ;-) of all IE users are on Windows. If you have an example that works in IE7 on Windows, please attach it.
(In reply to comment #13) > The primary use case for 'outline' is indicating focus. Sure, but the outline property itself is generic. I think it would be a mistake for a UA vendor to implement it with only :focus in mind and limit its usefulness for anything else. (Have this been discussed on www-style? I searched the archives but couldn't find anything.) My guess is that web designers will regard this as a bug. > For indicating focus, I think drawing > visible outlines at the position of covered elements is better. Why? Is this for some security concern, that focused form controls should render its outline on top? If so, I think there are other solutions to that problem.
(In reply to comment #15) > (In reply to comment #12) > > IE most certainly does implement outline. I doubt they've removed it from IE7. > > I tested again several examples in IE7 on Vista and XPSP2 and I couldn't > find any example that worked. I guess you're right. I assumed it supported 'outline' based on the outlines it draws for focused elements. But even if it doesn't support the property, the outlines it draws for focused elements are still relevant for compatibility.
Attached file Testcase #3 (deleted) —
IE7 on Vista draws the focus ring under the positioned block.
Fine. Get Hixie to explain why the spec says what it says.
(In reply to comment #16) > My guess is that web designers will regard this as a bug. So tell the designers that 'outline' is for focus outlines, and they should use borders instead.
Hixie suggests putting outlines in a new layer between layers 7 and 8. This is not quite what Mats said in comment #11. For example, <div style="width:50px; height:50px; outline:50px solid green;"></div> <div style="width:50px; height:50px; background:red;"></div> Drawing outlines in step 7.3 would draw the background over the outline. Drawing outlines in a new layer between 7 and 8 would draw the outline over the background. Which alternative is the interoperable one? I can change this fairly easily once we decide what the required behaviour is.
Flags: blocking1.9?
Status: RESOLVED → REOPENED
Resolution: INVALID → ---
Attached file Testcase #4 (deleted) —
Steps to Reproduce: 1. Load the attached test case. 2. Press tab to activate the link. 3. The link border appears through the yellow div. 4. Click a blank part of the page to deactivate the link. Now, right click the link. 5. The border again appears through the yellow div. Actual Results: The link border is not masked through the higher z-index opaque div. Expected Results: The link border should be covered by the higher z-index opaque div, just like its text is.
Flags: blocking1.9? → blocking1.9+
Attached patch fix (deleted) — Splinter Review
The fix is actually really simple. I'm very proud of the display list architecture :-) Mats, I'm throwing this review to you to keep it off David's plate and also because you raised the bug :-)
Assignee: nobody → roc
Status: REOPENED → ASSIGNED
Attachment #272131 - Flags: superreview?(mats.palmgren)
Attachment #272131 - Flags: review?(mats.palmgren)
OK it was originally Jesse's bug but you joined in :-)
Attached file reference (deleted) —
This makes a nice reftest along with the previous testcase.
Attached file Testcase #7 (deleted) —
Here's a better testcase for testing IE7. It looks like their themed <input> is actually changing its border color rather than using an "outline". They do display a focus ring for elements with tabindex="1" which is probably better for reverse engineering purposes. The case you mentioned in comment 22 is the green block. It turns out that IE7 and Webkit does outlines in a step between 7/8 as you suspected (the green block's outline is above the cyan area). Opera (9.20 on Vista) and Firefox 1.5/2 does it in 7.3. The attached patch makes us compatible with IE7/Webkit AFAICT. (well, IE7 paints its outline just inside the padding edge but it looks compatible layering-wise)
Comment on attachment 272131 [details] [diff] [review] fix > // 5: floats > resultList.AppendToTop(set.Floats()); >- // 6: general content >+ // 7: general content For completeness, I think you should include step 6 in the comments, I guess it should say "6,7: general content"? You might also want to change the order of these two lines: http://bonsai.mozilla.org/cvsblame.cgi?file=/mozilla/layout/generic/nsFrame.cpp&rev=3.741&root=/cvsroot&mark=1343-1344#1309 (not that it really matters, but they were listed in the same order as the paint order below)
Attachment #272131 - Flags: superreview?(mats.palmgren)
Attachment #272131 - Flags: superreview+
Attachment #272131 - Flags: review?(mats.palmgren)
Attachment #272131 - Flags: review+
Have we posted to www-style about the discrepancy in UA behavior here?
Checked in patch and reftest.
Status: ASSIGNED → RESOLVED
Closed: 19 years ago18 years ago
Resolution: --- → FIXED
Adding "in-testsuite+" based on roc's comment 33.
Flags: in-testsuite+
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: