Outline applied on a multi-line element should encompass all the line boxes and stay connected (join outlines together)
Categories
(Core :: Layout, defect)
Tracking
()
Webcompat Priority | P3 |
People
(Reporter: aaronlev, Unassigned)
References
(Blocks 3 open bugs)
Details
(4 keywords, Whiteboard: [layout:backlog:quality])
Attachments
(5 files)
One an element is broken up into several layout frames, the CSS outline should be drawn around the entire collection of frames. It should not be drawn as several overlapping outlines. This needs to take into account -moz-outline-radius and -moz-outline-offset as well.
Reporter | ||
Comment 1•19 years ago
|
||
There are some spec issues here, I think. How should the outline be drawn in a situation like this? <style>.vot::first-line { opacity:0.5; }</style> <div class="vot"> <span style="outline:2px solid red;">Hello<br>Kitty</span> </div> So we still need to break up the unified outline into pieces that belong to individual frames and paint each piece when its frame paints. But we need to make sure that no part of the unified outline is painted twice or we'll get incorrect results for rgba() colors. We may also need to think about how dotted or dashed borders are going to work. I thought this interacted with z-ordering too, but as far as I can tell it really doesn't. So in PaintOutline, we look at the other frames for the content and figure out what the union outline is and which part this frame should paint. Here's how this could work. There are two regions associated with each outline: the interior (the area enclosed by the outline's inner edge) and the exterior (the area enclosed by the outline's outer edge). Say that a frame paints its regular outline clipped to exclude all the exteriors of its prev-in-flow frames' outlines and to exclude all the interiors of its next-in-flow frames' outlines. This will guarantee that no point in any interior is painted, and every point in the union of the outlines is painted exactly once. I think we could actually implement it this way by building clip regions, even for outline-radius by gluing together 1-pixel-high rectangles. It might be a bit slow for frames with a large outline-radius but we could optimize by only doing the clipping thing when we know the frame's overflow area intersects with other frames in its in-flow list.
That doesn't address getting dots and dashes right. That seems really tricky. To fix that, we'd have to do the clipping thing but instead of painting the frame's outline normally, compute the geometric path for the union outline and paint that. That would require taking the post-segmentation polygon for each frame outline, figuring out intersections and which segments are the boundary (difficult since it has width), and then painting that.
Comment 4•19 years ago
|
||
The outline for an element should be rendered all at once, not piecemeal. So there isn't a problem here. You'd render the outline for that inline way after you've rendered the rest of the block (in fact you should do it at the end of the stacking context).
But in that example, the first line of the <span> element is in a different stacking context from the second line.
![]() |
||
Comment 6•19 years ago
|
||
*** Bug 280087 has been marked as a duplicate of this bug. ***
Comment 7•19 years ago
|
||
On the 19/6/05 nightly, rendering has improved for the testcase in this bug, but still fails for the testcase in bug 280087 (attachment 172646 [details]), as shown in this collection of screenshots. Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8b2) Gecko/20050619 Firefox/1.0+
Now that bug 317375 has landed, outlines for all frames for a single element are painted consecutively in z-order whenever possible. We could copy the merging logic from nsDisplayOpacity to nsDisplayOutline, or use some other approach, to ensure that the outlines for a single element are painted in a single operation. Cairo drawing APIs are available through Thebes in cairo builds, so we should be able to go ahead and fix this now in cairo builds. The only remaining problem is to figure out exactly how this should be implemented in its full generality. The idea in comment #2 would be fairly easy to do with cairo but it doesn't handle dotted and dashed outlines well.
Comment 9•18 years ago
|
||
Out of interest, is this likely to fix bug 328193?
Comment 10•18 years ago
|
||
I don't know about bug 328193, but speaking of dependencies, I'm pretty sure it should fix bug 337274 :-)
Comment 11•17 years ago
|
||
Guys is there any patch available for this.
Comment 12•15 years ago
|
||
Comment 14•14 years ago
|
||
IE8, Safari and Opera do this.
Comment 15•14 years ago
|
||
(The second example in testcase3 is probably another bug)
Updated•14 years ago
|
Updated•14 years ago
|
Updated•14 years ago
|
Updated•13 years ago
|
Comment 18•12 years ago
|
||
Testcase from CSS2.1 test suite ------------------------------- http://test.csswg.org/suites/css2.1/latest/html4/outline-layout-004.htm http://test.csswg.org/suites/css2.1/nightly-unstable/html4/outline-layout-004.htm Relevant chunk of spec ---------------------- " Outlines may be non-rectangular. For example, if the element is broken across several lines, the outline is the minimum outline that encloses all the element's boxes. In contrast to borders, the outline is not open at the line box's end or start, but is always fully connected if possible. " 18.4 Dynamic outlines: the 'outline' property http://www.w3.org/TR/CSS21/ui.html#dynamic-outlines
Comment 19•11 years ago
|
||
The inaccuracy of the bug summary makes it difficult to find this bug report; "glob" is not a real english verb, otherwise, it is a word difficult to understand or to translate. Bug summary changed to: Outline applied on a multi-line element should encompass all the line boxes and stay connected. I considered "wrap around", "enclose" and "surround". ------------- (In reply to j.j. (inactive in 2012) from comment #15) > (The second example in testcase3 is probably another bug) Such exaple is interesting because there is no test for such kind of line box scenario at CSS 2.1 test suite: a line box with a few inline boxes of various line-heights. I'm working on a test to be submitted based on your 2nd example in testcase3. When the spec says "Outlines may be non-rectangular. For example, if the element is broken across several lines, the outline is the minimum outline that encloses all the element's boxes.", then I think it suggests that a line box with several inline boxes of various line-heights should be enclosed using a non-rectangular outline. Gérard
Comment 20•11 years ago
|
||
> (...) there is no test for such kind of line > box scenario at CSS 2.1 test suite: a line box with a few inline boxes of > various line-heights. > I'm working on a test to be submitted based on your 2nd example in > testcase3. Recently submitted test at CSS 2.1 test suite: http://test.csswg.org/suites/css2.1/nightly-unstable/html4/outline-layout-006.htm IE9, IE10, Opera 12.12 and Opera 12.50 pass this test. I'll create another bug report for this one. Gérard
Comment 21•11 years ago
|
||
> I'll create another bug report for this one. Bug 825058: Outline applied on element with a few inline boxes of various line-heights should be minimal and should encompass all the inline boxes and stay connected
Comment 23•8 years ago
|
||
With 'writing-mode' set to 'vertical-rl' http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/outline-inline-vrl-012.xht With 'writing-mode' set to 'vertical-lr' http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/outline-inline-vlr-013.xht Chrome 50.0.2661.94, IE11 and Edge pass these 2 tests.
Comment 24•7 years ago
|
||
For reference: attaching image capture displaying testcase3 rendered in current browsers.
Comment 25•7 years ago
|
||
Testcase from CSS2.1 test suite ------------------------------- http://test.csswg.org/suites/css2.1/latest/html4/outline-layout-004.htm http://test.csswg.org/suites/css2.1/nightly-unstable/html4/outline-layout-004.htm That test is now in the NeedsWork category because the shape of the outline border (non-rectangular) is not specified by the CSS2.x spec (and not even in CSS2 UI). Only the "stay connected" and "enclose all content" items are required by the CSS2.x spec. Therefore, I have removed the http://test.csswg.org/suites/css2.1/nightly-unstable/html4/outline-layout-006.htm test. The tests With 'writing-mode' set to 'vertical-rl' http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/outline-inline-vrl-012.xht With 'writing-mode' set to 'vertical-lr' http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/outline-inline-vlr-013.xht have been updated accordingly and will be submitted soon.
Comment 26•7 years ago
|
||
> the shape of the outline
> border (non-rectangular) is not specified by the CSS2.x spec (and not even
> in CSS2 UI).
I meant to say
(...) is not specified by the CSS2.x spec (and not even in CSS3 UI).
![]() |
||
Updated•6 years ago
|
![]() |
||
Comment 27•6 years ago
|
||
Mass bug change to replace various 'parity' whiteboard flags with the new canonical keywords. (See bug 1443764 comment 13.)
Comment 30•4 years ago
|
||
Carrying on conversation from bug 1624061.
(In reply to Emilio Cobos Álvarez (:emilio) from comment #4)
I was thinking of making an nsDisplayOutline just on the first continuation and make it responsible for drawing all the other outlines, but that seems that it may not play well with invalidation...
Matt, do we have a precedent for something like this?
Indeed invalidation can be quite a pain for cases like this, and we really prefer to have each display item be based on the layout/style data from a single frame for that reason.
There are however indeed a few cases where we deal with this sort of thing.
One is nsDisplayItemBase::GetDependentFrame, which lets you register a second frame from which data is read. The DL invalidation code will handle invalidating the item if either of the frames change. It's possible that we could extend this to support a list of frames, but DL code can be very perf sensitive (due to giant lists), so adding size or indirections can be painful.
Another example is invalidating items for border-collapse tables. For this, the various table component frames override nsIFrame::InvalidateFrame, and propagate the invalidation up to their parent so that it eventually invalidates the root of the table. We could do something similar by adding a check in nsIFrame::MarkNeedsDisplayItemRebuild which checks if we had outlines as last paint (state bit?), and are a continuation, and if so also invalidate the primary frame. The same performance concerns apply.
(In reply to Miko Mynttinen [:miko] from comment #5)
We do display item merging by putting them together in one temporary item[1]. Based on the comment, it seems this was also meant to be used by nsDisplayOutline. Merging happens at the display list iteration level, in FlattenedDisplayListIterator[2].
This code is very messy because it previously required mutating the display items, which we cannot do with retained display lists. There is also a second problem with retained display lists: the display item ordering can change during display list merging, which means that we cannot rely on items staying consecutive (it should still be the common case). As it is, the code only works for container based display items.
If it's not too hard to extend the merging code to support leaf items, then I think it might be worth trying to support merging for outlines, given that we're already paying the complexity cost of supporting merging for other items (and have no real path forward to removing that).
Maybe this "grouping of display items" is better done during display list building or display list merging rather than painting?
As above, created merged items in the retained list means that we need to handle invalidating them correctly when any of the input frames changes, which isn't super fun.
I think it'd also be interesting to look at whether it's possible to merge during the Gecko DL -> WR DL conversion, or on the WR deserialization/scene building side.
Updated•4 years ago
|
Updated•3 years ago
|
![]() |
||
Updated•2 years ago
|
![]() |
||
Updated•2 years ago
|
Updated•1 year ago
|
Updated•1 year ago
|
Comment 32•1 year ago
|
||
The severity field for this bug is relatively low, S3. However, the bug has 8 duplicates.
:dholbert, could you consider increasing the bug severity?
For more information, please visit auto_nag documentation.
Comment 33•1 year ago
|
||
The last needinfo from me was triggered in error by recent activity on the bug. I'm clearing the needinfo since this is a very old bug and I don't know if it's still relevant.
Description
•