Github fragment links don't work

VERIFIED FIXED in Firefox 22

Status

()

Core
Layout
VERIFIED FIXED
5 years ago
5 years ago

People

(Reporter: Dan Wolff, Assigned: mats)

Tracking

({regression})

22 Branch
mozilla24
x86
All
regression
Points:
---
Bug Flags:
in-testsuite ?

Firefox Tracking Flags

(firefox21 unaffected, firefox22+ verified, firefox23+ verified, firefox24+ verified, firefox-esr17 unaffected)

Details

Attachments

(2 attachments, 1 obsolete attachment)

(Reporter)

Description

5 years ago
User Agent: Mozilla/5.0 (X11; Linux i686; rv:23.0) Gecko/20130519 Firefox/23.0 (Nightly/Aurora)
Build ID: 20130519004019

Steps to reproduce:

Go to e.g. https://github.com/mozilla/rust#readme


Actual results:

The top of the page is shown.


Expected results:

The Readme text should have been scrolled into the screen.

Many/all fragment links are affected:
* https://github.com/mozilla/rust/issues/1#issuecomment-711645
* https://github.com/mozilla/rust/wiki#rust

There is at least one type of fragment link that *does* work:
* https://github.com/mozilla/rust/blob/master/COPYRIGHT#L1

I believe that Firefox 22 too is affected.

Comment 1

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


Inlocal build
Last Good: a809066fbda9
First bad: a8f58efc2619

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
Status: UNCONFIRMED → NEW
status-firefox21: --- → unaffected
status-firefox-esr17: --- → unaffected
tracking-firefox22: --- → ?
tracking-firefox23: --- → ?
tracking-firefox24: --- → ?
Component: Untriaged → Layout
Ever confirmed: true
Keywords: regression
OS: Linux → All
Product: Firefox → Core
Version: 23 Branch → 22 Branch

Comment 2

5 years ago
Created attachment 751647 [details]
Reduced archive. open  http://127.0.0.1/bug873926/index.html#readme

Reproduce with http: scheme.
(not reproduce with file: scheme)

Comment 3

5 years ago
Comment on attachment 751647 [details]
Reduced archive. open  http://127.0.0.1/bug873926/index.html#readme

Sorry, the attachment 751647 [details] causes different behavior. maybe another bug.
Attachment #751647 - Attachment is obsolete: true
(Assignee)

Comment 4

5 years ago
Created attachment 751803 [details]
stacks

There's a style change on the root element which restores the state
and sets the mDidHistoryRestore flag on the root frame.
This occurs *before* nsHtml5TreeOpExecutor::DidBuildModel calls
ScrollToRef which is then blocked from scrolling due to the flag.
(Assignee)

Comment 5

5 years ago
Created attachment 751822 [details] [diff] [review]
fix

This works for me locally, without regressing bug 849219.

https://tbpl.mozilla.org/?tree=Try&rev=8af2cfeb933f

Not sure how to write a test for this...
Assignee: nobody → matspal
Attachment #751822 - Flags: review?(roc)

Updated

5 years ago
status-firefox22: --- → affected
status-firefox23: --- → affected
status-firefox24: --- → affected
tracking-firefox22: ? → +
tracking-firefox23: ? → +
tracking-firefox24: ? → +
https://hg.mozilla.org/mozilla-central/rev/2133f018b1c3
Status: NEW → RESOLVED
Last Resolved: 5 years ago
status-firefox24: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla24
(Reporter)

Comment 8

5 years ago
Are there any plans to backport this for Fx 22 and 23?

Comment 9

5 years ago
(In reply to Dan Wolff from comment #8)
> Are there any plans to backport this for Fx 22 and 23?

It's tracked to consider doing so, as long as the risk of a more critical regression is minimal.
(Assignee)

Comment 10

5 years ago
Comment on attachment 751822 [details] [diff] [review]
fix

[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 849219
User impact if declined: scrolling to fragment doesn't occur in some cases
Testing completed (on m-c, etc.): on m-c since 2013-05-22
Risk to taking this patch (and alternatives if risky): low risk
String or IDL/UUID changes made by this patch: none
Attachment #751822 - Flags: approval-mozilla-beta?
Attachment #751822 - Flags: approval-mozilla-aurora?
Comment on attachment 751822 [details] [diff] [review]
fix

Approving since it's low risk and still early enough in the cycle.
Attachment #751822 - Flags: approval-mozilla-beta?
Attachment #751822 - Flags: approval-mozilla-beta+
Attachment #751822 - Flags: approval-mozilla-aurora?
Attachment #751822 - Flags: approval-mozilla-aurora+
(Assignee)

Comment 12

5 years ago
https://hg.mozilla.org/releases/mozilla-aurora/rev/f326e14be7d5
https://hg.mozilla.org/releases/mozilla-beta/rev/1d3508fa8497
status-firefox22: affected → fixed
status-firefox23: affected → fixed
Verified fixed with Firefox 22 beta 3, build ID: 20130528181031, on Ubuntu 12.10 32bit and Mac OSX 10.8.3 32bit mode.
status-firefox22: fixed → verified
QA Contact: manuela.muntean
Verified Fixed using FF23b6 on Mac OS 10.8, Ubuntu 13.04 x86 and Windows 7 x64:
Build ID: 20130714030202
status-firefox23: fixed → verified
The fix is verified on Fx 24b1 too. BuildID: 20130806104538
Status: RESOLVED → VERIFIED
status-firefox24: fixed → verified
You need to log in before you can comment on or make changes to this bug.