Closed Bug 857336 Opened 8 years ago Closed 8 years ago

Anchor links don't scroll after going back

Categories

(Core :: Layout, defect)

22 Branch
defect
Not set
normal

Tracking

()

VERIFIED FIXED
mozilla23
Tracking Status
firefox22 + fixed
firefox23 --- verified

People

(Reporter: mrbkap, Assigned: mats)

References

Details

(Keywords: regression)

Attachments

(2 files)

Steps to reproduce:

1. Open https://bugzilla.mozilla.org/show_bug.cgi?id=839467
2. Click on the attachment "Part 1 - Remove JSContext pushing in AdoptNode. v2" to navigate to the patch.
3. Click on the back button to navigate back to the bug.
4. Click on the link "2013-04-02 14:48 PDT" under the originally-clicked link to go to the comment where the attachment was added to the bug.

Actual results:

* The URL bar updates to include the anchor.
* The forward button disappears

Expected results:

* In addition to the previous two actual results, we should have scrolled to the comment in question.

Of note:

* Clicking on the anchor link again scrolls correctly.
I should also note that I can reproduce this on Mozilla/5.0 (X11; Linux x86_64; rv:22.0) Gecko/20130401 Firefox/22.0
Leaving this on the nom list for now. Bug 851445 was deemed as low impact for users, and we still haven't found major user impact. If this ends up being a dupe, we'll likely untrack as well (unless people think this points to a larger issue).

We'd still accept a low risk fix for bug 851445 when ready.
bent can still reproduce this with a Windows nightly from 2013-04-10.
I can reproduce the scroll to anchor problem on comment#0.
http://hg.mozilla.org/mozilla-central/rev/ee5ca214e87c
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:23.0) Gecko/20130410 Firefox/23.0 ID:20130410065939


Regression window(m-c)
Good:
http://hg.mozilla.org/mozilla-central/rev/6d587302645a
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:22.0) Gecko/20130316 Firefox/22.0 ID:20130316145831
Bad:
http://hg.mozilla.org/mozilla-central/rev/0b052daa913c
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:22.0) Gecko/20130316 Firefox/22.0 ID:20130316160554
Pushlog;
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=6d587302645a&tochange=0b052daa913c



Regression window(m-i)
Good:
http://hg.mozilla.org/integration/mozilla-inbound/rev/fa0ee26e949c
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:22.0) Gecko/20130315 Firefox/22.0 ID:20130315153029
Bad:
http://hg.mozilla.org/integration/mozilla-inbound/rev/501804a40144
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:22.0) Gecko/20130315 Firefox/22.0 ID:20130315153228
Pushlog;
http://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=fa0ee26e949c&tochange=501804a40144

Suspected: Bug 851485
Blocks: 851485
No longer depends on: 851485
Suspected bug is wrong in comment #5

There are two regression

#1 regression
 Anchor links don't scroll after going back if the page scrolled before

STR
 1. Open https://bugzilla.mozilla.org/show_bug.cgi?id=839467
 1-2. Scroll to Down few pixels
 2. Click on the link "Part 1 - Remove JSContext pushing in AdoptNode. v2" in Attachment section
 3. Navigate back
 4. Click on the link "2013-04-02 14:48 PDT" under the originally-clicked link

Regression Wndow(m-i)
Good:
http://hg.mozilla.org/integration/mozilla-inbound/rev/5c174f3aed3c
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:22.0) Gecko/20130305 Firefox/22.0 ID:20130305032549
Bad:
http://hg.mozilla.org/integration/mozilla-inbound/rev/bd280c980a1b
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:22.0) Gecko/20130305 Firefox/22.0 ID:20130305042649
Pushlog:
http://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=5c174f3aed3c&tochange=bd280c980a1b

#1 Regressed by:
 bd280c980a1b	Mats Palmgren — Bug 842853 - Scroll position lost after page reload on url with anchor. r=roc


#2 regression
 Anchor links don't scroll after going back regardless of a scroll position

STR
 1. Open https://bugzilla.mozilla.org/show_bug.cgi?id=839467
 1-2. Regardless of a scroll position
 2. Click on the link "Part 1 - Remove JSContext pushing in AdoptNode. v2" in Attachment section
 3. Navigate back
 4. Click on the link "2013-04-02 14:48 PDT" under the originally-clicked link

Regression window(m-i)
Good:
http://hg.mozilla.org/integration/mozilla-inbound/rev/fa0ee26e949c
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:22.0) Gecko/20130315 Firefox/22.0 ID:20130315153029
Bad:
http://hg.mozilla.org/integration/mozilla-inbound/rev/501804a40144
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:22.0) Gecko/20130315 Firefox/22.0 ID:20130315153228
Pushlog;
http://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=fa0ee26e949c&tochange=501804a40144

#2 Regressed by:
 a8f58efc2619	Mats Palmgren — Bug 849219 - Store the scroll state also when the position is 0,0 to avoid scrolling to an #ID on reload. r=roc
Blocks: 849219
No longer blocks: 851485
To reproduce the problem, you maybe need to clear cache by "Clear Recent History".
Mats - do you think this bug may end up being worse for users than bug 842853 and bug 849219? Neither of those issues were user critical, so if not, this wouldn't be either.
Assignee: nobody → matspal
Adding needsinfo on :mats to help with comment #8
Flags: needinfo?(matspal)
Dropping qawanted since it appears the request for a regression window has been fulfilled. Please re-add if there's something we can do to help.
Keywords: qawanted
It seems hard to reproduce in a default profile, you have to navigate away
from the page a dozen or so pages, then going back to click the anchor link.

But I can reproduce the STR reliably after setting
browser.sessionhistory.max_total_viewers = 0
Flags: needinfo?(matspal)
Attached file stack
Here's the stack when we set "aScroll = false" in PresShell::GoToAnchor
http://hg.mozilla.org/mozilla-central/annotate/f8d27fe5d7c0/layout/base/nsPresShell.cpp#l2860
Attached patch fix?Splinter Review
Perhaps we should always allow scrolls coming from nsDocShell::ScrollToAnchor?
I don't know this docshell code very well...

https://tbpl.mozilla.org/?tree=Try&rev=fbf8207c354f
Attachment #739658 - Flags: feedback?(bugs)
Attachment #739658 - Flags: review?(roc)
Landed to get some baking time while waiting for feedback:
https://hg.mozilla.org/integration/mozilla-inbound/rev/f2e3cbbd3616
https://hg.mozilla.org/mozilla-central/rev/f2e3cbbd3616
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla23
Comment on attachment 739658 [details] [diff] [review]
fix?

[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 842853
User impact if declined: scrolling the viewport when clicking an #anchor link does not occur in some cases
Testing completed (on m-c, etc.): on m-c since 2013-04-21
Risk to taking this patch (and alternatives if risky): low risk
String or IDL/UUID changes made by this patch: none
Attachment #739658 - Flags: approval-mozilla-aurora?
Needinfo on Olli to help with the feedback needed in comment# 13 before approving on aurora.
Flags: needinfo?(bugs)
Comment on attachment 739658 [details] [diff] [review]
fix?

Make sure we don't regress bug 583889.
Attachment #739658 - Flags: feedback?(bugs) → feedback+
Flags: needinfo?(bugs)
We have a mochitest for bug 583889.  I checked the STR in bug 638598:
it still doesn't scroll - as expected. (Linux64 Nightly)
Attachment #739658 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Keywords: verifyme
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:23.0) Gecko/20100101 Firefox/23.0
Mozilla/5.0 (Windows NT 6.2; WOW64; rv:23.0) Gecko/20100101 Firefox/23.0
Mozilla/5.0 (X11; Linux i686; rv:23.0) Gecko/20100101 Firefox/23.0
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.9; rv:23.0) Gecko/20100101 Firefox/23.0

Verified as fixed on Firefox 23 beta 8 (buildID: 20130722172257) and latest Nightly (buildID: 20130723030205).
Status: RESOLVED → VERIFIED
Keywords: verifyme
You need to log in before you can comment on or make changes to this bug.