Scripts from topix.net apparently causing page to scroll down when opening

RESOLVED FIXED in mozilla2.0b7

Status

()

Core
Editor
RESOLVED FIXED
7 years ago
7 years ago

People

(Reporter: WildcatRay, Assigned: Away for a while)

Tracking

({regression, testcase})

Trunk
mozilla2.0b7
regression, testcase
Points:
---
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(blocking2.0 final+)

Details

Attachments

(3 attachments, 4 obsolete attachments)

(Reporter)

Description

7 years ago
Apparently, scripts from topix.net are causing the page to scroll down opening them. 

Examples:
http://www.dispatch.com/live/content/local_news/stories/2010/10/18/motorist-killed-when-car-police-cruiser-collide.html?sid=101
http://www.dispatch.com/live/content/local_news/stories/2010/10/18/18-bus-school-close.html?sid=101
http://www.dispatch.com/live/content/local_news/stories/2010/10/18/18-dump-dem.html?sid=101

It also appears to happen on this link:
http://newsbusters.org/blogs/tim-graham/2010/10/18/wapo-promotes-gospel-according-jimmy-they-find-carters-preachy-know-it-a

I cannot tell if they use topix scripts or just scripts similar to those on topix.

If you open any of these in Minefield/Trunk, the pages will open and then scroll down. Pages should only open. (I did not see this on IE9.)

I have tested in both Win7 and WinXP using the 32-bit builds in both safe mode and using a fairly new, no add-ons installed profile. Others have confirmed this bug, too. I think this started with the compartments landing.

The nightly build of 13-Oct does not show the problem. The nightly of 14-Oct does.
(Reporter)

Comment 1

7 years ago
FYI-May need to be logged in to the newsbusters.org web site to see this behavior.

Comment 2

7 years ago
Regression window:
Works:
http://hg.mozilla.org/mozilla-central/rev/b829cf5debce
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0b8pre) Gecko/20101012 Firefox/4.0b8pre ID:20101013074132
Fails:
http://hg.mozilla.org/mozilla-central/rev/178f26e21cfc
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0b8pre) Gecko/20101011 Firefox/4.0b8pre ID:20101013081930
Pushlog:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=b829cf5debce&tochange=178f26e21cfc

Updated

7 years ago
blocking2.0: --- → ?
Keywords: regression
Target Milestone: --- → Firefox 4.0

Comment 3

7 years ago
Probably bug 597331

Comment 4

7 years ago
Happens on Linux too.
http://hg.mozilla.org/mozilla-central/rev/cfd18201f49b
Mozilla/5.0 (X11; Linux i686; rv:2.0b8pre) Gecko/20101018 Firefox/4.0b8pre ID:20101018030619

In local build,
Build from 178f26e21cfc : fails
Build from 18d0545c5f0e : works
So, Landing of the following cset causes the problem.
changeset 178f26e21cfc	Ehsan Akhgari — Bug 597331 - Reframing a textarea sets the caret position to the end of its contents; r=bzbarsky a=blocking-final+
Blocks: 597331
Component: General → Editor
OS: Windows 7 → All
Product: Firefox → Core
QA Contact: general → editor
Hardware: x86_64 → All
Target Milestone: Firefox 4.0 → mozilla2.0
(Assignee)

Updated

7 years ago
Assignee: nobody → ehsan
Keywords: testcase-wanted

Comment 5

7 years ago
Created attachment 484084 [details]
semi-reduced html

Updated

7 years ago
Attachment #484084 - Attachment is obsolete: true

Comment 6

7 years ago
Created attachment 484305 [details]
reduced sample

CSS file should be external CSS file.

Strange to say,
If CSS is written in script tag, the scroll does not happen.
And If removed SCRIPT tag, the scroll does not happen.
(Assignee)

Comment 7

7 years ago
Created attachment 484698 [details]
Reduced testcase

Here's the most reduced testcase for this bug.
Attachment #484305 - Attachment is obsolete: true
(Assignee)

Updated

7 years ago
Keywords: testcase-wanted → testcase
(Assignee)

Comment 8

7 years ago
Created attachment 484858 [details] [diff] [review]
Patch (v1)

Please note that this requires an API change (so that we can pass SCROLL_FIRST_ANCESTOR_ONLY to nsIPresShell::ScrollFrameRectIntoView if needed.  Can this land post b7, or on the b7 relbranch as well?
Attachment #484858 - Flags: review?(roc)
Attachment #484858 - Flags: approval2.0?
Instead of one (or multiple) PRBool parameters, can we use a flags word please?
(Assignee)

Comment 10

7 years ago
Created attachment 485113 [details] [diff] [review]
Patch (v2)

(In reply to comment #9)
> Instead of one (or multiple) PRBool parameters, can we use a flags word please?

Of course.
Attachment #484858 - Attachment is obsolete: true
Attachment #485113 - Flags: review?(roc)
Attachment #485113 - Flags: approval2.0?
Attachment #484858 - Flags: review?(roc)
Attachment #484858 - Flags: approval2.0?
You're still adding a boolean parameter to nsSelection::ScrollIntoView, which now has three boolean parameters. Can we flagize them too please?
(Assignee)

Comment 12

7 years ago
Created attachment 485222 [details] [diff] [review]
Part 2: flagize nsTypedSelection::ScrollIntoView

(In reply to comment #11)
> You're still adding a boolean parameter to nsSelection::ScrollIntoView, which
> now has three boolean parameters. Can we flagize them too please?

Sure.
Attachment #485222 - Flags: review?(roc)
Attachment #485222 - Flags: approval2.0?
Attachment #485222 - Flags: review?(roc) → review+
Attachment #485113 - Flags: review?(roc) → review+
blocking2.0: ? → final+
(Assignee)

Updated

7 years ago
Whiteboard: [needs landing]
(Assignee)

Updated

7 years ago
Attachment #485113 - Flags: approval2.0?
(Assignee)

Updated

7 years ago
Attachment #485222 - Flags: approval2.0?
(Assignee)

Comment 13

7 years ago
Created attachment 485383 [details] [diff] [review]
Part 2: flagize nsTypedSelection::ScrollIntoView

I had made a mistake in part 2 of this patch.  In nsTypedSelection::ScrollSelectionIntoViewEvent::Run, I had practically removed the aDoFlush and aIsSynchronous parameters and wasn't setting the corresponding flags.  This version of the patch fixes that problem.
Attachment #485222 - Attachment is obsolete: true
Attachment #485383 - Flags: review?(roc)
(Assignee)

Updated

7 years ago
Whiteboard: [needs landing]
Attachment #485383 - Flags: review?(roc) → review+
(Assignee)

Updated

7 years ago
Whiteboard: [needs landing]

Comment 14

7 years ago
This bug happens for me in this page: http://es.paperblog.com/25-anos-de-back-to-the-future-primera-parte-el-de-lorean-206615/

It's working fine on Chrome, IE9 and Opera.
(Assignee)

Comment 15

7 years ago
http://hg.mozilla.org/mozilla-central/rev/8cb3a9d9f7f9
http://hg.mozilla.org/mozilla-central/rev/e4400c2c3a3d
Status: NEW → RESOLVED
Last Resolved: 7 years ago
Flags: in-testsuite+
Keywords: dev-doc-needed
Resolution: --- → FIXED
Whiteboard: [needs landing]
Target Milestone: mozilla2.0 → mozilla2.0b8
(Assignee)

Comment 16

7 years ago
I pushed a follow-up to fix a Thunderbird build bustage:

http://hg.mozilla.org/mozilla-central/rev/142c7b20fd8c
Duplicate of this bug: 605484
Target Milestone: mozilla2.0b8 → mozilla2.0b7
These API changes do not impact any currently-documented interfaces, so I'm removing the doc needed keyword. These changes will be picked up when these interfaces get documented in the future.
Keywords: dev-doc-needed
You need to log in before you can comment on or make changes to this bug.