Closed
Bug 1420722
Opened 7 years ago
Closed 7 years ago
nsIFrame::GetFlattenedTreeParentPrimaryFrame doesn't account for display: contents.
Categories
(Core :: Web Painting, defect, P3)
Core
Web Painting
Tracking
()
RESOLVED
FIXED
mozilla59
| Tracking | Status | |
|---|---|---|
| firefox59 | --- | fixed |
People
(Reporter: emilio, Assigned: emilio)
References
Details
Attachments
(1 file)
Bug 1359709 introduced it instead of walking the parent frame. But our flattened tree parent doesn't necessarily have a primary frame if it has display: contents.
Looks to me like this should just walk the frame tree skipping anon boxes?
|
||
Comment 1•7 years ago
|
||
We'd have to walk up the frame tree via placeholders to maintain the current behaviour.
We really need to spec to be clarified for what to do here (especially [1] and [2]), then we can fix our behaviour to match.
[1] https://github.com/w3c/csswg-drafts/issues/1944
[2] https://github.com/w3c/csswg-drafts/issues/1952
|
|
||
Updated•7 years ago
|
Priority: -- → P3
| Comment hidden (mozreview-request) |
|
Assignee | |
Updated•7 years ago
|
Attachment #8942873 -
Flags: review?(matt.woodrow)
|
|
||
Comment 3•7 years ago
|
||
| mozreview-review | ||
Comment on attachment 8942873 [details]
Bug 1420722: Just use GetInFlowParent for now.
https://reviewboard.mozilla.org/r/213134/#review219058
Attachment #8942873 -
Flags: review?(matt.woodrow) → review+
Pushed by ecoal95@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/5d33d01409e3
Just use GetInFlowParent for now. r=mattwoodrow
|
|
Assignee | |
Comment 5•7 years ago
|
||
Matt, any idea of a good test I could steal for this?
Flags: needinfo?(matt.woodrow)
|
|
||
Comment 6•7 years ago
|
||
https://bugzilla.mozilla.org/show_bug.cgi?id=1359709#c4 has the broken reftest that made me write the original patch, I think you could include display:contents into that.
Flags: needinfo?(matt.woodrow)
Pushed by ecoal95@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/86deafd2d502
Fix test indentation. r=whitespace-only
https://hg.mozilla.org/integration/autoland/rev/743d5e003df9
Test this. r=me
|
|
Assignee | |
Comment 8•7 years ago
|
||
(In reply to Matt Woodrow (:mattwoodrow) from comment #6)
> https://bugzilla.mozilla.org/show_bug.cgi?id=1359709#c4 has the broken
> reftest that made me write the original patch, I think you could include
> display:contents into that.
Thanks! I confirmed that https://hg.mozilla.org/integration/autoland/rev/743d5e003df9 makes it fail without the patch.
|
||
Comment 9•7 years ago
|
||
| bugherder | ||
https://hg.mozilla.org/mozilla-central/rev/5d33d01409e3
https://hg.mozilla.org/mozilla-central/rev/86deafd2d502
https://hg.mozilla.org/mozilla-central/rev/743d5e003df9
Status: NEW → RESOLVED
Closed: 7 years ago
status-firefox59:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla59
|
||
Updated•7 years ago
|
Assignee: nobody → emilio
You need to log in
before you can comment on or make changes to this bug.
Description
•