fix followup issues related to position:sticky on fragmented elements

RESOLVED FIXED in mozilla31

Status

()

defect
RESOLVED FIXED
6 years ago
5 years ago

People

(Reporter: dbaron, Assigned: kip)

Tracking

Trunk
mozilla31
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

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 FIXME for GetUsedBorderAndPadding / GetUsedMargin is bug 916302.
Indeed it is.  Thanks.
Assignee

Updated

5 years ago
Assignee: nobody → kgilbert
Assignee

Comment 4

5 years ago
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();
Assignee

Comment 5

5 years ago
(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?
Flags: needinfo?(dbaron)
Assignee

Comment 6

5 years ago
Posted patch bug920688.patch (obsolete) — Splinter Review
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.)
Flags: needinfo?(dbaron)
Attachment #8410568 - Flags: review?(dbaron) → review-
Assignee

Comment 8

5 years ago
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 #8410568 - Attachment is obsolete: true
Attachment #8410587 - Flags: review?(dbaron)
Attachment #8410587 - Flags: review?(dbaron) → review+
Assignee

Updated

5 years ago
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/20eb244eae3b
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla31
You need to log in before you can comment on or make changes to this bug.