266968 - [FIXr]Auto offsets wrong for abs pos kid of block kid of a rel pos inline

Status: RESOLVED FIXED

(Core :: Layout: Positioned, defect, P1)

Description

[Boris Zbarsky [:bzbarsky]]

In brief, the CalculateHypotheticalBox code in nsHTMLReflowState doesn't handle
the case of a relatively positioned inline having a block kid with an abs pos
descendant, because in this case the cb and the block are not ancestors of each
other.

Comment 1

[Boris Zbarsky [:bzbarsky]]

Attachment: There should be no red

Comment 2

[Boris Zbarsky [:bzbarsky]]

Attachment: Patch

Comment 3

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 164105 [details] [diff] [review]
Patch

roc, would you review?	This makes the following changes:

1)  Adds two new methods to nsLayoutUtils (the other changes here are
header-fanout cleanup I already had in my tree that I couldn't easily 
disentangle; this is a good chance to check it in  :) ).

2)  Makes nsHTMLReflowState use the new code (that fixes this bug).

3)  Clarifies some code in nsCSSFrameConstructor, since it really doesn't need
to deal with absolutely positioned elements now that reflow is fixed.

4)  Fixes various callers in the tree to use the new methods.

Of particular note in #4 are the changes to nsGfxScrollFrame, which actually
change the functionality.  The code without these changes is a no-op; that
seems to be a regression introduced in revision 3.126 of the file...

For the rest, I think the big danger is getting signs wrong... that's something
to expecially watch for when reviewing this patch.

Comment 4

[Boris Zbarsky [:bzbarsky]]

Oh, one more thing.  There is another common pattern around -- walking up to the
first widget, then converting to screen coordinates... I didn't really mess with
those callers, but maybe we want a helper for that too.

Comment 5

[Boris Zbarsky [:bzbarsky]]

Attachment: Updated to handle scrollviews right

Comment 6

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 164135 [details] [diff] [review]
Updated to handle scrollviews right

Roc and I talked about this some, and decided on a slightly different api.

Comment 7

[Boris Zbarsky [:bzbarsky]]

Attachment: Better yet api, as discussed

Comment 8

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 164146 [details] [diff] [review]
Better yet api, as discussed

Comment 3 still describes the patch, with the change to the nsIFrame/nsIView
member method api.

Comment 9

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

Comment on attachment 164146 [details] [diff] [review]
Better yet api, as discussed

+  return offset;
+
+}

Stray blank line.

accessible isn't linked to gklayout, so won't linking to nsIFrame::GetOffsetTo
fail? Maybe we need a virtual nsIFrame::GetOffsetToExternal.

Other than that, looks fantastic.

Comment 10

[Boris Zbarsky [:bzbarsky]]

Yeah, you're right.  I'll add a GetOffsetToInternal() before checking in.

Comment 11

[Boris Zbarsky [:bzbarsky]]

Fix checked in, with those changes.

Comment 12

[Alfred Peng]

The testcase shows partly red again on the trunk: Mozilla/5.0 (X11; U; SunOS sun4u; en-US; rv:1.9a1) Gecko/20060516 Minefield/3.0a1.

Is it a regression?

Comment 13

[Boris Zbarsky [:bzbarsky]]

No, the testcase isn't quite right...  I could create one that is, I guess.  Compare the rendering in a November 3, 2004 build, though.

Comment 14

[Boris Zbarsky [:bzbarsky]]

Attachment: Fixed testcase