Closed Bug 1237825 Opened 5 years ago Closed 5 years ago

Find the root scroll frame even if the root element doesn't have a primary frame

Categories

(Core :: Layout, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla46
Tracking Status
firefox46 --- fixed

People

(Reporter: tnikkel, Assigned: tnikkel)

Details

Attachments

(1 file)

This is a weird and tricky case.
Attached patch patchSplinter Review
Typical structure is

viewportframe mContent=null
  htmlscrollframe mContent=rootelement
    canvasframe mContent=rootelement
      blockframe mContent=rootelement
        ...


with rootelement->GetPrimaryFrame() == blockframe

But if the blockframe isn't created because it is displaynone (or it has a failed -moz-binding) we get

viewportframe mContent=null
  htmlscrollframe mContent=rootelement
    canvasframe mContent=rootelement

with rootelement->GetPrimaryFrame() == null

So we just need to be able to get the presshell given the content node only.
Attachment #8705397 - Flags: review?(mstange)
Comment on attachment 8705397 [details] [diff] [review]
patch

Review of attachment 8705397 [details] [diff] [review]:
-----------------------------------------------------------------

r+ assuming you add the null check or have a good justification for why it's not necessary.

::: layout/base/nsLayoutUtils.cpp
@@ +857,5 @@
>    nsIFrame* frame = aContent->GetPrimaryFrame();
> +  if (aContent->OwnerDoc()->GetRootElement() == aContent) {
> +    nsIPresShell* presShell = frame ? frame->PresContext()->PresShell() : nullptr;
> +    if (!presShell) {
> +      presShell = aContent->OwnerDoc()->GetShell();

presShell can still be null after this line, right? Don't you need to null check it before using it below?
Attachment #8705397 - Flags: review?(mstange) → review+
I'll add the null. Probably wasn't bitter by it because we likely only call it if we have a frame, and at least a presshell.
https://hg.mozilla.org/mozilla-central/rev/980596816271
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla46
Product: Core → Core Graveyard
Component: Layout: Misc Code → Layout
Product: Core Graveyard → Core
You need to log in before you can comment on or make changes to this bug.