Closed
Bug 873926
Opened 12 years ago
Closed 12 years ago
Github fragment links don't work
Categories
(Core :: Layout, defect)
Tracking
()
VERIFIED
FIXED
mozilla24
Tracking | Status | |
---|---|---|
firefox21 | --- | unaffected |
firefox22 | + | verified |
firefox23 | + | verified |
firefox24 | + | verified |
firefox-esr17 | --- | unaffected |
People
(Reporter: 326374, Assigned: MatsPalmgren_bugz)
References
Details
(Keywords: regression)
Attachments
(2 files, 1 obsolete file)
(deleted),
text/plain
|
Details | |
(deleted),
patch
|
roc
:
review+
lsblakk
:
approval-mozilla-aurora+
lsblakk
:
approval-mozilla-beta+
|
Details | Diff | Splinter Review |
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•12 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•12 years ago
|
||
Reproduce with http: scheme.
(not reproduce with file: scheme)
Comment 3•12 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•12 years ago
|
||
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•12 years ago
|
||
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•12 years ago
|
Attachment #751822 -
Flags: review?(roc) → review+
Assignee | ||
Comment 6•12 years ago
|
||
Flags: in-testsuite?
Comment 7•12 years ago
|
||
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla24
Comment 9•12 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•12 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 11•12 years ago
|
||
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•12 years ago
|
||
Comment 13•12 years ago
|
||
Verified fixed with Firefox 22 beta 3, build ID: 20130528181031, on Ubuntu 12.10 32bit and Mac OSX 10.8.3 32bit mode.
QA Contact: manuela.muntean
Comment 14•11 years ago
|
||
Verified Fixed using FF23b6 on Mac OS 10.8, Ubuntu 13.04 x86 and Windows 7 x64:
Build ID: 20130714030202
Comment 15•11 years ago
|
||
The fix is verified on Fx 24b1 too. BuildID: 20130806104538
Status: RESOLVED → VERIFIED
You need to log in
before you can comment on or make changes to this bug.
Description
•