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

RESOLVED FIXED

Status

()

Core
Layout: R & A Pos
P2
normal
RESOLVED FIXED
8 years ago
7 years ago

People

(Reporter: Yannick Lavoquer, Assigned: tnikkel)

Tracking

({regression})

Trunk
x86
All
regression
Points:
---

Firefox Tracking Flags

(blocking2.0 final+)

Details

(URL)

Attachments

(3 attachments)

(Reporter)

Description

8 years ago
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
(Reporter)

Updated

8 years ago
Keywords: regression
Version: unspecified → Trunk
range based of comment 0:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=120667a01fd2&tochange=a43e2f7eda8f
blocking2.0: --- → ?

Comment 2

7 years ago
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

Comment 3

7 years ago
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

Comment 4

7 years ago
Created attachment 441894 [details]
minimzed testcase

Ok now I got the missing piece: overflow:hidden.

Updated

7 years ago
Keywords: testcase-wanted
blocking2.0: ? → final+
Priority: -- → P2
(Assignee)

Updated

7 years ago
Assignee: nobody → tnikkel
(Assignee)

Comment 5

7 years ago
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?
Yeah, I think both.
(Assignee)

Comment 7

7 years ago
Created attachment 483389 [details] [diff] [review]
patch
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+
(Assignee)

Comment 9

7 years ago
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.
(Assignee)

Comment 10

7 years ago
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.
Bug 601352 will fix that issue, right?
(Assignee)

Comment 12

7 years ago
The patch in bug 601352 has no effect on the problem in comment 10.
(Assignee)

Comment 13

7 years ago
Created attachment 492156 [details] [diff] [review]
fix test

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
(Assignee)

Comment 15

7 years ago
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 :)
(Assignee)

Comment 17

7 years ago
http://hg.mozilla.org/mozilla-central/rev/3c680a3b987f
Status: NEW → RESOLVED
Last Resolved: 7 years ago
Resolution: --- → FIXED

Comment 18

7 years ago
Not fixed for this site: http://climateprogress.org/2010/11/17/confusionist-judith-curry-wicked-martin-weitzman/#comment-307396
(Assignee)

Comment 19

7 years ago
Please file a new bug for that with steps to reproduce and cc me, thanks!

Comment 20

7 years ago
Done: https://bugzilla.mozilla.org/show_bug.cgi?id=614909
You need to log in before you can comment on or make changes to this bug.