Closed Bug 920688 Opened 7 years ago Closed 7 years ago
fix followup issues related to position:sticky on fragmented elements
In patch 9a of bug 828312, I note some FIXMEs that are followups to bug 904197. We should probably fix these issue before enabling position:sticky, or at least investigate them further.
The comments are added in: https://hg.mozilla.org/integration/mozilla-inbound/rev/8ba2dc63f1bf
The FIXME for GetUsedBorderAndPadding / GetUsedMargin is bug 916302.
Indeed it is. Thanks.
The remaining FIXME is within StickyScrollContainer::ComputeStickyLimits: // FIXME (Bug 920688): cbFrame isn't quite right if we're dealing // with a block-in-inline split whose first part is a block. We // probably want the first in flow of the containing block of the // first inline part. (Or maybe those block-in-inline split pieces // are never a containing block, and we're ok?) nsIFrame* cbFrame = aFrame->GetContainingBlock();
(In reply to :kip (Kearwood Gilbert) from comment #4) > The remaining FIXME is within StickyScrollContainer::ComputeStickyLimits: > > // FIXME (Bug 920688): cbFrame isn't quite right if we're dealing > // with a block-in-inline split whose first part is a block. We > // probably want the first in flow of the containing block of the > // first inline part. (Or maybe those block-in-inline split pieces > // are never a containing block, and we're ok?) > nsIFrame* cbFrame = aFrame->GetContainingBlock(); As GetNearestBlockContainer (nsFrame.cpp) iterates upwards through the parents to find the block container, it is skipping frames that could not be a block container. Among those conditions is that the frame does not have a type of nsIFrame::eLineParticipant. Would it be safe to assume that this would prevent a block-in-inline split piece from being returned as a containing block?
This patch applies the correction as described in the FIXME comment; however, I have not yet found a test case that results in a different value for cbFrame than the original aFrame->GetcontainingBlock() call on its own. If my hypothesis in Comment 5 is true, then perhaps this patch will not be required and the FIXME comment needs to be updated instead.
Attachment #8410568 - Flags: review?(dbaron)
It looks like the function GetNearestBlockContainer in nsFrame.cpp (helper for nsIFrame::GetContainingBlock) already handles this, so the FIXME can just be removed. (Note that the patch isn't quite implementing what the comment suggests, since it's getting the first part rather than the first inline part, but it seems like we don't need to worry about that.)
Attachment #8410568 - Flags: review?(dbaron) → review-
This patch removes the FIXME comment from StickyScrollContainer::ComputeStickyLimits, as the condition described in the FIXME is already handled by GetNearestBlockContainer in nsFrame.cpp
Attachment #8410587 - Flags: review?(dbaron) → review+
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla31
You need to log in before you can comment on or make changes to this bug.