408226 - Scrolling to anchor does not work when IFRAME is before anchor

Status: NEW

(Core :: Layout, defect)

Description

[Michael Hohner]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; de; rv:1.9b1) Gecko/2007110904 Firefox/3.0b1
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; de; rv:1.9b1) Gecko/2007110904 Firefox/3.0b1

Go to

http://spiders.mhohner.de/stats.php#findlist

to reproduce. Firefox scrolls only to the map, which is an IFRAME. But the referenced anchor is further down, at the next heading.

You can also go to the top of the page and click on the "Fundliste" link. Same result.

Reproducible: Always

Steps to Reproduce:
1. Go to http://spiders.mhohner.de/stats.php#findlist

Actual Results:  
Scrolls to IFRAME.

Expected Results:  
Scroll to named anchor.

Also happens with Firefox 2.0.0.11 and IE 7. No problem with Opera 9.22.

Comment 1

[Mats Palmgren (inactive)]

Attachment: Testcase #1

Comment 2

[Mats Palmgren (inactive)]

Attachment: Testcase #2

Comment 3

[Mats Palmgren (inactive)]

Attachment: Testcase #3

Comment 4

[Mats Palmgren (inactive)]

Attachment: Testcase #4

Comment 5

[Mats Palmgren (inactive)]

Attachment: Testcase #5

Comment 6

[Mats Palmgren (inactive)]

Attachment: Frame dump (URL)

Comment 7

[Mats Palmgren (inactive)]

Attachment: Patch rev. 1

The problem with the URL is that the target anchor is a block-in-inline,
causing an IB split chain where the first frame is empty and thus fits
next to the IFRAME on the line above where the perceived link content is.
(see the Frame dump)

I think it makes sense to include all non-empty frames in an IB split
chain when calculating the union rect of the frames.

Comment 8

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

Why don't you just call nsLayoutUtils::GetNextContinuationOrSpecialSibling instead of GetNextContinuation?

Comment 9

[Mats Palmgren (inactive)]

Attachment: Testcase #6

Comment 10

[Mats Palmgren (inactive)]

Attachment: Patch rev. 2

GetNextContinuationOrSpecialSibling() version.
This patch does not fix Testcase #6 though.

Comment 11

[Mats Palmgren (inactive)]

Attachment: Patch rev. 3

It's the helper function UnionRectForClosestScrolledView() that explicitly
adds the empty frame rects.  An alternative solution would be to only
include non-empty frame rects (instead of using nsIFrame::IsEmpty()),
but still use the position of an empty frame rect if we can't find a
non-empty one.  This patch also fixes Testcase #6.

Both patches pass mochitests (which covers bug 388019 and bug 66619).
Feel free to pick the one you prefer.

Comment 12

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

Hmm, could it make sense to use GetClientRects here or at least share code with it?

Comment 13

[Michael Hohner]

This bug is still not fixed. It is also present in Firefox 3.0. Will it ever work?

Comment 14

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

Mats, would you like to use nsLayoutUtils::GetAllInFlowRects here?

Comment 15

[BugBot [:suhaib / :marco/ :calixte]]

The bug assignee is inactive on Bugzilla, so the assignee is being reset.