Closed Bug 539949 Opened 14 years ago Closed 14 years ago

Regression : Clicking some links with anchors doesn't scroll downward their destinations [allocine.com]

Categories

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

x86
All
defect

Tracking

()

RESOLVED FIXED
Tracking Status
blocking2.0 --- final+

People

(Reporter: djobi.djoba, Assigned: tnikkel)

References

()

Details

(Keywords: regression)

Attachments

(3 files)

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; fr; rv:1.9.3a1pre) Gecko/20100115 Minefield/3.7a1pre
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; fr; rv:1.9.3a1pre) Gecko/20100115 Minefield/3.7a1pre

Verified with a clean profile.
Platform : Linux & windows XP

Reproducible: Always

Steps to Reproduce:
1.Load http://www.allocine.fr/seance/salle_gen_csalle=C0179.html
2.Click any link in section "Films de la semaine" 
Actual Results:  
Updated URL in location bar. Nothing else happens.

Expected Results:  
Should have jumped downward the right part of the document

It works fine when styles are disabled [View > Pages Style > No Style]

Regression Window :
Good : Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.3a1pre) Gecko/20100111 Minefield/3.7a1pre
Built from http://hg.mozilla.org/mozilla-central/rev/120667a01fd2
 
Broken : Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.3a1pre) Gecko/20100112 Minefield/3.7a1pre
Built from http://hg.mozilla.org/mozilla-central/rev/a43e2f7eda8f
Keywords: regression
Version: unspecified → Trunk
Based on the range, it's probably bug 526394. Confirming, since I can reproduce this on Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.3a5pre) Gecko/20100425 Minefield/3.7a5pre and can't find a similar-sounding issue in bug 526394's duplicates.

Anyone wants to try creating a simple testcase for this? https://developer.mozilla.org/en/Reducing_testcases
Blocks: 526394
Status: UNCONFIRMED → NEW
Ever confirmed: true
Keywords: testcase-wanted
From my poking around with this it seems to involve positioning.  With the dom inspector it seems like the anchor is on the page with 3.6 but with trunk it doesn't seem like it's on the page. With DOMi in 3.6 you select the anchor div and it will scroll into view.  On the trunk no scrolling occurs.

One key CSS rule seems to be this one:
.anchor { position:absolute; position:relative\0; top:-22px; }  
The computed style for an anchor that I looked at in both versions in absolute.

My first attempt at reducing to a testcase resulted in something that doesn't work in either trunk or 3.6.  So I guess I'm missing something.
Component: General → Layout: R & A Pos
QA Contact: general → layout.r-and-a-pos
Attached file minimzed testcase
Ok now I got the missing piece: overflow:hidden.
Keywords: testcase-wanted
blocking2.0: ? → final+
Priority: -- → P2
Assignee: nobody → tnikkel
So the problem here is in that in PresShell::ScrollFrameRectIntoView if we weren't able to scroll to make any of the target rect visible we bail out, and the target rect in this case comes from an abspos child of an overflow hidden container that is positioned outside of the visible area of the parent.

Changeset 5d8894904869 changed the behaviour here. We used to 1) not bail like above, and 2) not intersect the target rect with the scrollport after scrolling it. The simplest thing to fix this bug is probably just to not bail, and maybe not intersect as well?
Attached patch patchSplinter Review
Attachment #483389 - Flags: review?(roc)
Comment on attachment 483389 [details] [diff] [review]
patch

The code is OK but something is wrong with your reftest; the test and reference are identical and neither of them triggers scrolling into view?
Attachment #483389 - Flags: review?(roc) → review+
Good catch. The test and reference are different, the test has <div style="position: relative; overflow:hidden;">, the ref has <div style="position: relative;">. The reftest.list file should have "#test2" appended to both the test and reference (I was using a temp reftest.list file to test that it failed without the patch and worked with and didn't copy from it, but instead just wrote the line anew and forgot that part.) I will make those changes in my queue so it is actually a valid test.
This causes toolkit/content/tests/widgets/test_mousecapture.xul to fail because it uses some synth mouse moves to drag and select some text and scroll an iframe in the test. But this patch causes every all scrollable ancestors to scroll where they didn't, even the the root content document scroll frame that contains the iframe the test is run in. I'll probably fix it by just resetting the scroll position of the things we don't want scrolled.
The patch in bug 601352 has no effect on the problem in comment 10.
Attached patch fix testSplinter Review
This is what I plan to land to fix the test fail.
Testcase in comment 0 and comment 4 works with latest build:
Mozilla/5.0 (Windows NT 6.1; rv:2.0b8pre) Gecko/20101124 Firefox/4.0b8pre ID:20101124174809
That's probably because I landed this bug. I wait until I'm sure a push will stick before marking the relevant bugs resolved, which is why I hadn't changed this bug yet.
I know what you mean and didn't want to get ahead of you, I figured you'd probably update once your ready :)
http://hg.mozilla.org/mozilla-central/rev/3c680a3b987f
Status: NEW → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Please file a new bug for that with steps to reproduce and cc me, thanks!
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: