All users were logged out of Bugzilla on October 13th, 2018

CSS outline is not coalesced in dynamic case

RESOLVED FIXED

Status

()

--
minor
RESOLVED FIXED
9 years ago
9 years ago

People

(Reporter: jruderman, Assigned: bzbarsky)

Tracking

(Blocks: 1 bug, {testcase})

Trunk
x86
Mac OS X
testcase
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(3 attachments, 1 obsolete attachment)

(Reporter)

Description

9 years ago
Created attachment 410317 [details]
testcase (dynamic)

Based on layout/reftests/bugs/424236-5.html.  Probably a regression.
(Reporter)

Comment 1

9 years ago
Created attachment 410318 [details]
reference
(Assignee)

Comment 3

9 years ago
Yes.  The key part is that the two testcases have different frame trees.

I guess I can actually make this case (the trailing inline is empty and a block is the first thing appended) generate the same frame tree as the static case.  Let me give that a shot.
(Assignee)

Comment 4

9 years ago
Hmm.  So here's a question about the continuation model.  Is it possible to have an inline whose first continuation has no kids but which has some other continuation that does have kids?
(Assignee)

Comment 5

9 years ago
Created attachment 413782 [details] [diff] [review]
Fix, assuming the answer to comment 4 is "yes"

Though I suspect that if that answer is "yes" some of our other append code might not be quite correct...  so I hope it's actually "no".
Assignee: nobody → bzbarsky
Status: NEW → ASSIGNED
Attachment #413782 - Flags: review?(roc)
(Assignee)

Comment 7

9 years ago
Created attachment 413791 [details] [diff] [review]
"no" == simpler fix
Attachment #413782 - Attachment is obsolete: true
Attachment #413791 - Flags: review?(roc)
Attachment #413782 - Flags: review?(roc)
(Assignee)

Comment 8

9 years ago
Though I guess the other approach might be faster if we have lots of continuations for that trailing inline.....  whichever one you like more?
Why didn't the outline merging code detect this case, anyway?  Should it have?  (Should we be fixing this bug in two different ways?)
Possibly.  How should outline merging generally work for ib splits?  Where do we merge outlines, in general?
I suppose the little unioning we do (which isn't nearly what we should) is in nsCSSRendering::PaintOutline, about 20 lines in.
Ah.  That's not actual unioning; that's just drawing the outline around a different rect.  It seems to me that we don't actually coalesce outlines in a sane way at all.  Consider:

data:text/html,<!DOCTYPE html><span style="outline: 5px solid black">aaa<br>bbb</span>
Pushed http://hg.mozilla.org/mozilla-central/rev/1126b08afa75
Status: ASSIGNED → RESOLVED
Last Resolved: 9 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
(Assignee)

Updated

9 years ago
Depends on: 535721
You need to log in before you can comment on or make changes to this bug.