Closed Bug 873926 Opened 6 years ago Closed 6 years ago
Github fragment links don't work
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.
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
Status: UNCONFIRMED → NEW
Component: Untriaged → Layout
Ever confirmed: true
OS: Linux → All
Product: Firefox → Core
Version: 23 Branch → 22 Branch
Reproduce with http: scheme. (not reproduce with file: scheme)
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
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.
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)
Attachment #751822 - Flags: review?(roc) → review+
Are there any plans to backport this for Fx 22 and 23?
(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.
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
Comment on attachment 751822 [details] [diff] [review] fix Approving since it's low risk and still early enough in the cycle.
Verified fixed with Firefox 22 beta 3, build ID: 20130528181031, on Ubuntu 12.10 32bit and Mac OSX 10.8.3 32bit mode.
Verified Fixed using FF23b6 on Mac OS 10.8, Ubuntu 13.04 x86 and Windows 7 x64: Build ID: 20130714030202
The fix is verified on Fx 24b1 too. BuildID: 20130806104538
You need to log in before you can comment on or make changes to this bug.