232780 - nsViewManager::CreateDisplayList parameters aX/aY used inconsistently

Status: RESOLVED FIXED

(Core :: Web Painting, defect, P1)

Description

[Robert O'Callahan (:roc) (email my personal email if necessary)]

The aX and aY parameters to nsViweManager::CreateDisplayList are used
inconsistently. Sometimes we add them to the result of aView->GetBounds(), or
add them to the result of the old nsView::GetClippedRect (which returned a rect
relative to the origin of aView's parent) --- indicating that they should be an
offset to the origin of aView's parent. However, it's clear that in the call to
CreateDisplayList from BuildDisplayList, aX and aY are the offset from the
topmost root view (the view with no parent) to displayRoot (NOT displayRoot's
parent). In fact there's code "if (aTopView == aView)" in CreateDisplayList that
compensates for this fact by adjusting "bounds" and "pos". But this code does
not correct aX and aY so errors can still creep in later (as it turns out, only
if displayRoot is not the topmost root view, i.e., only when we're painting a
floating view or handling event capturing).

This is why the "if (!clipped) clipRect = aClipRect;" code is 'needed' in
AddToDisplayList ... it wipes out the incorrect clipRect computed by this bug.

Comment 1

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Attachment: fix

This patch makes aX/aY consistently represent an offset to the origin of
aView's parent. It also adds some much needed comments.

With this patch, the offensive code in AddToDisplayList can be deleted.

Comment 2

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 140346 [details] [diff] [review]
fix

r+sr=dbaron, but why not remove the |aTopView| parameter entirely?

Comment 3

[Robert O'Callahan (:roc) (email my personal email if necessary)]

the other patch I just submitted to you adds a new use of aTopView in the body
of CreateDisplayList.

Comment 4

[Robert O'Callahan (:roc) (email my personal email if necessary)]

But in general CreateDisplayList could use some cleanup, yeah.

Comment 5

[Robert O'Callahan (:roc) (email my personal email if necessary)]

checked in